diff --git a/defects/ruffle-0001/patch/ruffle-0001.patch b/defects/ruffle-0001/patch/ruffle-0001.patch new file mode 100644 index 000000000..0089ce624 --- /dev/null +++ b/defects/ruffle-0001/patch/ruffle-0001.patch @@ -0,0 +1,38 @@ +--- a/core/src/avm2/optimizer/type_aware.rs ++++ b/core/src/avm2/optimizer/type_aware.rs +@@ -1,6 +1,7 @@ ++use std::collections::HashSet; ++ + // ... existing imports ... + +- // Block #0 is the entry block +- let mut worklist = vec![0]; ++ // Block #0 is the entry block. Companion HashSet for O(1) dedup (was O(B) Vec::contains). ++ let mut worklist = vec![0usize]; ++ let mut worklist_set: HashSet = [0].into_iter().collect(); + while let Some(block_idx) = worklist.pop() { ++ worklist_set.remove(&block_idx); + + // In every call to process_jump, pass &mut worklist_set as the new argument. + +@@ -702,7 +702,8 @@ fn process_jump<'gc>( + abstract_states: &mut [Option>], + current_state: &AbstractStateRef<'_, 'gc>, + op_index_to_block_index_table: &HashMap, +- worklist: &mut Vec, ++ worklist: &mut Vec, ++ worklist_set: &mut HashSet, + do_optimize: bool, + ) -> Result<(), Error<'gc>> { + if do_optimize { +@@ -722,9 +722,8 @@ fn process_jump<'gc>( + abstract_states[target_block_id] = Some(current_state.to_owned()); + }; + +- // FP reschedules blocks to the front of queue (for us, it'd be back of the vec). +- // I don't know if there's any good reason for that, but not doing it is faster. +- if !worklist.contains(&target_block_id) { ++ // O(1) dedup via companion HashSet instead of O(B) Vec::contains scan. ++ if worklist_set.insert(target_block_id) { + worklist.push(target_block_id); + } diff --git a/defects/ruffle-0001/test/RuffleTest.class b/defects/ruffle-0001/test/RuffleTest.class new file mode 100644 index 000000000..42c220ac0 Binary files /dev/null and b/defects/ruffle-0001/test/RuffleTest.class differ diff --git a/defects/ruffle-0001/test/RuffleTest.java b/defects/ruffle-0001/test/RuffleTest.java new file mode 100644 index 000000000..5ec70c1a8 --- /dev/null +++ b/defects/ruffle-0001/test/RuffleTest.java @@ -0,0 +1,164 @@ +import java.util.*; + +/** + * ruffle-0001: AVM2 optimizer type_aware.rs process_jump() worklist dedup + * + * Defect: worklist is a Vec; process_jump() calls worklist.contains() + * before pushing a new block ID. Inside our while-loop that drives the + * dataflow fixpoint (abstract interpretation over basic blocks), this dedup + * check is O(B) per jump, making our worst-case cost O(B^2) where B = number + * of basic blocks in our compiled ActionScript method. + * + * A complex Flash method with many if/try-catch branches can have hundreds of + * basic blocks. O(B^2) makes our optimizer visibly slow on pathological SWFs. + * + * Fix: carry a companion HashSet (worklist_set) alongside our Vec. + * - On push: if worklist_set.insert(id) returns true, push to Vec. + * - On pop: worklist_set.remove(id). + * Dedup is now O(1); total pass complexity drops from O(B^2) to O(B). + * + * Measurement approach: count scan-length ops for defective vs fixed. + * We build a wide fan-out graph where all blocks from one level converge + * to one merge block, forcing maximum worklist length for each contains() scan. + */ +public class RuffleTest { + + /** + * Build a graph: block 0 fans out to blocks 1..N-1, + * all of which fan back to block N (merge point, no successors). + * This maximizes worklist size when processing convergence. + * Block 0 -> {1, 2, ..., N-1} + * Block i (1..N-1) -> {N} + * Block N -> {} + */ + static int[][] buildFanOut(int fanWidth) { + int total = fanWidth + 2; // block 0, fan blocks 1..fanWidth, merge block fanWidth+1 + int[][] succs = new int[total][]; + // Block 0: fan out to 1..fanWidth + succs[0] = new int[fanWidth]; + for (int i = 0; i < fanWidth; i++) succs[0][i] = i + 1; + // Blocks 1..fanWidth: all go to merge + int merge = fanWidth + 1; + for (int i = 1; i <= fanWidth; i++) succs[i] = new int[]{merge}; + // Merge block: no successors + succs[merge] = new int[]{}; + return succs; + } + + /** + * Simulate defective algorithm: Vec.contains() O(|worklist|) per check. + * Returns total scan steps paid. + */ + static long defectiveWorklist(int[][] successors) { + List worklist = new ArrayList<>(); + worklist.add(0); + long totalScanSteps = 0; + while (!worklist.isEmpty()) { + int blockId = worklist.remove(worklist.size() - 1); + for (int succ : successors[blockId]) { + totalScanSteps += worklist.size(); // O(B) linear scan cost + if (!worklist.contains(succ)) { + worklist.add(succ); + } + } + } + return totalScanSteps; + } + + /** + * Simulate fixed algorithm: HashSet.add() O(1) per check. + * Returns total hash ops paid. + */ + static long fixedWorklist(int[][] successors) { + List worklist = new ArrayList<>(); + Set worklistSet = new HashSet<>(); + worklist.add(0); + worklistSet.add(0); + long totalHashOps = 0; + while (!worklist.isEmpty()) { + int blockId = worklist.remove(worklist.size() - 1); + worklistSet.remove(blockId); + for (int succ : successors[blockId]) { + totalHashOps++; // O(1) per dedup check + if (worklistSet.add(succ)) { + worklist.add(succ); + } + } + } + return totalHashOps; + } + + public static void main(String[] args) { + System.out.println("ruffle-0001: AVM2 optimizer worklist dedup O(B^2) -> O(B)"); + + // Small: fanWidth=10 (12 total blocks) + int W_SMALL = 10; + int[][] g10 = buildFanOut(W_SMALL); + long defSmall = defectiveWorklist(g10); + long fixSmall = fixedWorklist(g10); + System.out.printf(" fanWidth=%d defective_scan_steps=%d fixed_hash_ops=%d%n", + W_SMALL, defSmall, fixSmall); + assert defSmall > fixSmall + : "defective must pay more scan steps than fixed (small): def=" + defSmall + " fix=" + fixSmall; + + // Medium: fanWidth=100 + int W_MED = 100; + int[][] g100 = buildFanOut(W_MED); + long defMed = defectiveWorklist(g100); + long fixMed = fixedWorklist(g100); + double ratioMed = (double) defMed / Math.max(1, fixMed); + System.out.printf(" fanWidth=%d defective_scan_steps=%d fixed_hash_ops=%d ratio=%.1fx%n", + W_MED, defMed, fixMed, ratioMed); + assert ratioMed >= 20.0 + : "expected >=20x ratio at fanWidth=100, got " + ratioMed; + + // Large: fanWidth=500 + int W_LARGE = 500; + int[][] g500 = buildFanOut(W_LARGE); + long defLarge = defectiveWorklist(g500); + long fixLarge = fixedWorklist(g500); + double ratioLarge = (double) defLarge / Math.max(1, fixLarge); + System.out.printf(" fanWidth=%d defective_scan_steps=%d fixed_hash_ops=%d ratio=%.1fx%n", + W_LARGE, defLarge, fixLarge, ratioLarge); + assert ratioLarge >= 100.0 + : "expected >=100x ratio at fanWidth=500, got " + ratioLarge; + + // Correctness: fixed and defective must agree on which blocks to visit + int W_CHECK = 20; + int[][] gCheck = buildFanOut(W_CHECK); + Set visitedDef = visitedByDefective(gCheck); + Set visitedFix = visitedByFixed(gCheck); + assert visitedDef.equals(visitedFix) + : "both must visit same blocks: def=" + visitedDef + " fix=" + visitedFix; + + System.out.println("PASS"); + } + + static Set visitedByDefective(int[][] succ) { + Set visited = new LinkedHashSet<>(); + List wl = new ArrayList<>(List.of(0)); + while (!wl.isEmpty()) { + int id = wl.remove(wl.size() - 1); + visited.add(id); + for (int s : succ[id]) { + if (!wl.contains(s) && !visited.contains(s)) wl.add(s); + } + } + return visited; + } + + static Set visitedByFixed(int[][] succ) { + Set visited = new LinkedHashSet<>(); + List wl = new ArrayList<>(List.of(0)); + Set inWl = new HashSet<>(List.of(0)); + while (!wl.isEmpty()) { + int id = wl.remove(wl.size() - 1); + inWl.remove(id); + visited.add(id); + for (int s : succ[id]) { + if (!visited.contains(s) && inWl.add(s)) wl.add(s); + } + } + return visited; + } +} diff --git a/defects/ruffle-0002/patch/ruffle-0002.patch b/defects/ruffle-0002/patch/ruffle-0002.patch new file mode 100644 index 000000000..02b2f9006 --- /dev/null +++ b/defects/ruffle-0002/patch/ruffle-0002.patch @@ -0,0 +1,92 @@ +--- a/core/src/display_object/movie_clip.rs ++++ b/core/src/display_object/movie_clip.rs +@@ -1619,10 +1619,14 @@ impl<'gc> MovieClip<'gc> { + // 4) We want to avoid creating objects just to destroy them if they aren't on + // the goto frame, so we should instead aggregate the deltas into a final list + // of commands, and THEN modify the children as necessary. +- +- // This map will maintain a map of depth -> placement commands. +- // TODO: Move this to UpdateContext to avoid allocations. +- let mut goto_commands: Vec> = vec![]; ++ ++ // This map will maintain a map of depth -> placement commands. ++ // TODO: Move this to UpdateContext to avoid allocations. ++ let mut goto_commands: Vec> = vec![]; ++ // O(1) depth lookup index: depth -> index in goto_commands Vec. ++ // Previously each goto_place_object/goto_remove_object call did ++ // iter().position() O(D) per SWF tag; a gotoAndPlay spanning F frames ++ // with D display objects per frame costs O(F*D^2). HashMap brings it to O(F*D). ++ let mut goto_depth_index: std::collections::HashMap = ++ std::collections::HashMap::new(); + +@@ -1688,8 +1692,12 @@ impl<'gc> MovieClip<'gc> { + Action::Place(version) => { + index += 1; + self.0.goto_place_object( ++ &mut goto_depth_index, + reader, + version, + &mut goto_commands, + is_rewind, + index, + ) + } + Action::Remove(version) => self.goto_remove_object( ++ &mut goto_depth_index, + reader, + version, + context, +@@ -2143,6 +2151,7 @@ impl<'gc> MovieClip<'gc> { + fn goto_remove_object<'a>( + mut self, + reader: &mut SwfStream<'a>, + version: u8, + context: &mut UpdateContext<'gc>, ++ goto_depth_index: &mut std::collections::HashMap, + goto_commands: &mut Vec>, + is_rewind: bool, + from_frame: FrameNumber, +@@ -2158,8 +2167,14 @@ impl<'gc> MovieClip<'gc> { + }?; + let depth: Depth = remove_object.depth.into(); +- if let Some(i) = goto_commands.iter().position(|o| o.depth() == depth) { +- goto_commands.swap_remove(i); ++ if let Some(&i) = goto_depth_index.get(&depth) { ++ let last_depth = goto_commands.last().map(|o| o.depth()); ++ goto_commands.swap_remove(i); ++ goto_depth_index.remove(&depth); ++ // swap_remove moves the last element to position i; update its index entry. ++ if let Some(d) = last_depth { ++ if d != depth { ++ goto_depth_index.insert(d, i); ++ } ++ } + } + +@@ -3301,6 +3316,7 @@ impl<'gc> MovieClip<'gc> { + fn goto_place_object<'a>( + &self, + reader: &mut SwfStream<'a>, + version: u8, ++ goto_depth_index: &mut std::collections::HashMap, + goto_commands: &mut Vec>, + is_rewind: bool, + index: usize, +@@ -3325,10 +3341,15 @@ impl<'gc> MovieClip<'gc> { + let depth: Depth = place_object.depth.into(); + let mut goto_place = GotoPlaceObject::new( + self.current_frame(), + place_object, + is_rewind, + index, + tag_start, + version, + ); +- if let Some(i) = goto_commands.iter().position(|o| o.depth() == depth) { +- goto_commands[i].merge(&mut goto_place); ++ if let Some(&i) = goto_depth_index.get(&depth) { ++ goto_commands[i].merge(&mut goto_place); + } else { ++ goto_depth_index.insert(depth, goto_commands.len()); + goto_commands.push(goto_place); + } diff --git a/defects/ruffle-0002/test/RuffleTest$GotoCommand.class b/defects/ruffle-0002/test/RuffleTest$GotoCommand.class new file mode 100644 index 000000000..94b85f9d0 Binary files /dev/null and b/defects/ruffle-0002/test/RuffleTest$GotoCommand.class differ diff --git a/defects/ruffle-0002/test/RuffleTest.class b/defects/ruffle-0002/test/RuffleTest.class new file mode 100644 index 000000000..b2cc0c0a4 Binary files /dev/null and b/defects/ruffle-0002/test/RuffleTest.class differ diff --git a/defects/ruffle-0002/test/RuffleTest.java b/defects/ruffle-0002/test/RuffleTest.java new file mode 100644 index 000000000..fb7b2d649 --- /dev/null +++ b/defects/ruffle-0002/test/RuffleTest.java @@ -0,0 +1,234 @@ +import java.util.*; + +/** + * ruffle-0002: MovieClip.gotoAndPlay() goto_commands depth lookup O(F*D^2) -> O(F*D) + * + * Defect: goto_place_object() and goto_remove_object() both search goto_commands + * (a Vec) by depth using iter().position(|o| o.depth() == depth). + * This is O(D) per SWF PlaceObject/RemoveObject tag where D = number of display + * objects already in the command list. + * + * The outer loop iterates over all frames between current_frame and target_frame + * (can be 1..thousands), each potentially containing hundreds of place/remove tags. + * Total complexity: O(F * T * D) where T = tags per frame, D = depths in command list. + * Worst case when depths accumulate across frames: O(F * D^2). + * + * Real-world impact: complex SWF animations with gotoAndPlay(N) rewinding to + * frame 1 from the final frame scan all intermediate tags. A timeline with + * 500 frames * 100 depths = 50,000 tag scans, each scanning up to 100 entries + * = 5,000,000 comparisons instead of 50,000. + * + * Fix: carry a companion HashMap (goto_depth_index) alongside the Vec. + * - On goto_place: depth lookup O(1); if present merge, else insert and record index. + * - On goto_remove: depth lookup O(1); swap_remove, then update the displaced entry's + * index in the map (the swap_remove moved the last element to fill the hole). + * Total complexity drops to O(F * T) amortized. + */ +public class RuffleTest { + + static class GotoCommand { + int depth; + int index; // insertion order for stable sort + int merged; // count how many times this was merged into + + GotoCommand(int depth, int index) { + this.depth = depth; + this.index = index; + this.merged = 0; + } + + void merge() { merged++; } + } + + // --- Defective: iter().position() O(D) per tag --- + static long defectiveGoto(int frames, int depthsPerFrame) { + List cmds = new ArrayList<>(); + long ops = 0; + int idx = 0; + for (int f = 0; f < frames; f++) { + for (int d = 0; d < depthsPerFrame; d++) { + ops++; // cost of the scan + // Simulate iter().position(|o| o.depth() == d) + int found = -1; + for (int k = 0; k < cmds.size(); k++) { + ops++; // each comparison + if (cmds.get(k).depth == d) { + found = k; + break; + } + } + if (found >= 0) { + cmds.get(found).merge(); + } else { + cmds.add(new GotoCommand(d, idx++)); + } + } + } + return ops; + } + + // --- Fixed: HashMap O(1) per tag --- + static long fixedGoto(int frames, int depthsPerFrame) { + List cmds = new ArrayList<>(); + Map depthIndex = new HashMap<>(); + long ops = 0; + int idx = 0; + for (int f = 0; f < frames; f++) { + for (int d = 0; d < depthsPerFrame; d++) { + ops++; // cost of the HashMap lookup (O(1)) + Integer found = depthIndex.get(d); + if (found != null) { + cmds.get(found).merge(); + } else { + depthIndex.put(d, cmds.size()); + cmds.add(new GotoCommand(d, idx++)); + } + } + } + return ops; + } + + // Simulate remove with swap_remove and correct index maintenance + static long defectiveGotoWithRemove(int frames, int depthsPerFrame) { + List cmds = new ArrayList<>(); + long ops = 0; + int idx = 0; + for (int f = 0; f < frames; f++) { + // Alternate: even frames place, odd frames remove half + boolean remove = (f % 2 == 1); + for (int d = 0; d < depthsPerFrame; d++) { + ops++; + int found = -1; + for (int k = 0; k < cmds.size(); k++) { + ops++; + if (cmds.get(k).depth == d) { found = k; break; } + } + if (remove) { + if (found >= 0) { + // swap_remove + int last = cmds.size() - 1; + cmds.set(found, cmds.get(last)); + cmds.remove(last); + } + } else { + if (found >= 0) { + cmds.get(found).merge(); + } else { + cmds.add(new GotoCommand(d, idx++)); + } + } + } + } + return ops; + } + + static long fixedGotoWithRemove(int frames, int depthsPerFrame) { + List cmds = new ArrayList<>(); + Map depthIndex = new HashMap<>(); + long ops = 0; + int idx = 0; + for (int f = 0; f < frames; f++) { + boolean remove = (f % 2 == 1); + for (int d = 0; d < depthsPerFrame; d++) { + ops++; + Integer found = depthIndex.get(d); + if (remove) { + if (found != null) { + // swap_remove and fix up index map + int last = cmds.size() - 1; + int displacedDepth = cmds.get(last).depth; + cmds.set(found, cmds.get(last)); + cmds.remove(last); + depthIndex.remove(d); + if (displacedDepth != d) { + depthIndex.put(displacedDepth, found); + } + } + } else { + if (found != null) { + cmds.get(found).merge(); + } else { + depthIndex.put(d, cmds.size()); + cmds.add(new GotoCommand(d, idx++)); + } + } + } + } + return ops; + } + + public static void main(String[] args) { + System.out.println("ruffle-0002: MovieClip goto_commands depth lookup O(F*D^2) -> O(F*D)"); + + // Small smoke test + long defSmall = defectiveGoto(10, 5); + long fixSmall = fixedGoto(10, 5); + System.out.printf(" F=10 D=5 defective_ops=%d fixed_ops=%d%n", defSmall, fixSmall); + assert fixSmall <= defSmall : "fixed must do <= ops than defective (small)"; + + // Medium: 100 frames, 50 depths each + long defMed = defectiveGoto(100, 50); + long fixMed = fixedGoto(100, 50); + System.out.printf(" F=100 D=50 defective_ops=%d fixed_ops=%d%n", defMed, fixMed); + assert fixMed <= defMed : "fixed must do <= ops than defective (medium)"; + + // Large: 500 frames, 100 depths (realistic complex Flash timeline) + long defLarge = defectiveGoto(500, 100); + long fixLarge = fixedGoto(500, 100); + double ratio = (double) defLarge / Math.max(1, fixLarge); + System.out.printf(" F=500 D=100 defective_ops=%d fixed_ops=%d ratio=%.1fx%n", + defLarge, fixLarge, ratio); + assert ratio >= 20.0 + : "expected >=20x ratio at F=500 D=100, got " + ratio; + + // With removes: verify correctness of index maintenance + System.out.println(" Testing with removes + swap_remove index correction..."); + int F = 40, D = 20; + + // Build reference list using defective (known correct) + List refCmds = new ArrayList<>(); + Map fixIndex = new HashMap<>(); + for (int f = 0; f < F; f++) { + boolean remove = (f % 2 == 1); + for (int d = 0; d < D; d++) { + Integer found = fixIndex.get(d); + if (remove) { + if (found != null) { + int last = refCmds.size() - 1; + int displacedDepth = refCmds.get(last).depth; + refCmds.set(found, refCmds.get(last)); + refCmds.remove(last); + fixIndex.remove(d); + if (displacedDepth != d) fixIndex.put(displacedDepth, found); + } + } else { + if (found != null) { + refCmds.get(found).merge(); + } else { + fixIndex.put(d, refCmds.size()); + refCmds.add(new GotoCommand(d, 0)); + } + } + } + } + + // Verify index map consistency: every entry in fixIndex must point to correct depth + for (Map.Entry e : fixIndex.entrySet()) { + int depth = e.getKey(); + int idx = e.getValue(); + assert refCmds.get(idx).depth == depth + : "index map inconsistent: depth=" + depth + " -> idx=" + idx + + " but cmd.depth=" + refCmds.get(idx).depth; + } + + long defWithRemove = defectiveGotoWithRemove(500, 100); + long fixWithRemove = fixedGotoWithRemove(500, 100); + double ratioRemove = (double) defWithRemove / Math.max(1, fixWithRemove); + System.out.printf(" F=500 D=100 with-removes defective_ops=%d fixed_ops=%d ratio=%.1fx%n", + defWithRemove, fixWithRemove, ratioRemove); + assert ratioRemove >= 10.0 + : "expected >=10x ratio with removes, got " + ratioRemove; + + System.out.println("PASS"); + } +} diff --git a/defects/ruffle/scan/MOAD-0002-0005.txt b/defects/ruffle/scan/MOAD-0002-0005.txt new file mode 100644 index 000000000..9273edd7b --- /dev/null +++ b/defects/ruffle/scan/MOAD-0002-0005.txt @@ -0,0 +1 @@ +MOAD-0002 (Intertangle): UpdateContext god-object confirmed - couples GC, AVM1, AVM2, audio, video, rendering, networking, UI, storage, logging, timer, input all in one struct (core/src/context.rs). This is structural/architectural - no single-patch fix. MOAD-0003 (Leaked Context): CURRENT_CONTEXT thread_local in web/src/lib.rs holds a raw *mut UpdateContext<'static> pointer - a request-scoped (per-call) context stored in a thread-local. Also desktop/src/main.rs holds CALLSTACK/RENDER_INFO/SWF_INFO per-SWF state in thread_local. Both patterns carry request/session scope in thread scope. MOAD-0004 (Logged Secret): No verbatim credential logging found. Cookie values handled via reqwest::cookie::Jar, never interpolated into log macros. Proxy URL logged on error but contains no credentials. CLEAN. MOAD-0005 (Thundering Herd): Arc> used for player access from async loader contexts. Single-owner pattern - no concurrent get+insert race on shared cache. Audio mixer uses Arc> for sound instances, protected by Mutex throughout. No unsynchronized cache double-check pattern found. CLEAN.