diff --git a/defects/vice-0001/patch/vice-0001-monitor-breakpoint-list-linear-scan.patch b/defects/vice-0001/patch/vice-0001-monitor-breakpoint-list-linear-scan.patch new file mode 100644 index 000000000..d5b2faffb --- /dev/null +++ b/defects/vice-0001/patch/vice-0001-monitor-breakpoint-list-linear-scan.patch @@ -0,0 +1,49 @@ +# UNDF: UNDF-2026-XXXXXXXXX +--- a/src/monitor/mon_breakpoint.c ++++ b/src/monitor/mon_breakpoint.c +@@ -349,16 +349,18 @@ static checkpoint_list_t *search_checkpoint_list(checkpoint_list_t *head, unsig + static checkpoint_list_t *search_checkpoint_list(checkpoint_list_t *head, unsigned loc) + { + checkpoint_list_t *cur_entry; ++ unsigned loc_only = addr_location((MON_ADDR)loc); + + cur_entry = head; + + /* The list is sorted in increasing order by start_addr. If the current +- entry is > than the search item, we can drop out early. ++ entry start address is already past loc, no later entry can match. + */ + while (cur_entry) { ++ if (addr_location(cur_entry->checkpt->start_addr) > loc_only) { ++ break; /* sorted list: no further entry can match */ ++ } + if (mon_is_in_range(cur_entry->checkpt->start_addr, + cur_entry->checkpt->end_addr, loc)) { + return cur_entry; + } +- + cur_entry = cur_entry->next; + } + + return NULL; + } +# +# CWE-407: search_checkpoint_list traverses the entire sorted checkpoint +# (breakpoint/watchpoint) linked list — O(N) per memory access — even +# though the list is kept in ascending start_addr order. The code +# comment promises early exit ("we can drop out early") but never +# implements it. +# +# monitor_check_breakpoints is called per opcode from the 6510/Z80/65816 +# core macros whenever MI_BREAK is set (any breakpoint exists). +# At N=50 breakpoints and 1 MHz emulated CPU that is 50 M comparisons/s. +# +# Fix: add one comparison before the range check. When the entry's +# start_addr already exceeds the target address the sorted invariant +# guarantees no later entry can match — break immediately. +# Result: O(log N) effective average for uniformly distributed accesses, +# O(1) for the common case where the PC is below the first breakpoint. +# +# Severity: MEDIUM — affects interactive debugging sessions only +# (MI_BREAK gate); with many breakpoints, per-cycle overhead is +# measurable on slow hosts or while profiling emulated code. diff --git a/defects/vice-0001/test/ViceMonitorBreakpointTest$Checkpoint.class b/defects/vice-0001/test/ViceMonitorBreakpointTest$Checkpoint.class new file mode 100644 index 000000000..13ddf62d6 Binary files /dev/null and b/defects/vice-0001/test/ViceMonitorBreakpointTest$Checkpoint.class differ diff --git a/defects/vice-0001/test/ViceMonitorBreakpointTest.class b/defects/vice-0001/test/ViceMonitorBreakpointTest.class new file mode 100644 index 000000000..12ff1230c Binary files /dev/null and b/defects/vice-0001/test/ViceMonitorBreakpointTest.class differ diff --git a/defects/vice-0001/test/ViceMonitorBreakpointTest.java b/defects/vice-0001/test/ViceMonitorBreakpointTest.java new file mode 100644 index 000000000..0cb4c58cf --- /dev/null +++ b/defects/vice-0001/test/ViceMonitorBreakpointTest.java @@ -0,0 +1,171 @@ +import java.util.*; + +/** + * Unit test for VICE CWE-407 defect vice-0001. + * + * vice-0001: search_checkpoint_list in mon_breakpoint.c traverses the + * entire sorted checkpoint linked list O(N) per memory access even though + * the list is kept in ascending start_addr order. The code comment says + * "we can drop out early" but never implements the early exit. + * + * Models the C linked-list scan and demonstrates the O(1)/O(log N) fix + * by adding a single guard: if entry.start > target, break immediately. + */ +public class ViceMonitorBreakpointTest { + + /** Represents one checkpoint (breakpoint/watchpoint). */ + static class Checkpoint { + int start_addr; + int end_addr; + + Checkpoint(int start, int end) { + this.start_addr = start; + this.end_addr = end; + } + } + + // --------------------------------------------------------------- + // DEFECTIVE: O(N) scan — no early exit even though list is sorted + // --------------------------------------------------------------- + + /** + * Models search_checkpoint_list as written: walks every node. + * Returns true if any checkpoint covers addr. + */ + static int[] defective_search(List sortedList, int addr) { + int comparisons = 0; + for (Checkpoint cp : sortedList) { + comparisons++; + if (addr >= cp.start_addr && addr <= cp.end_addr) { + return new int[]{1, comparisons}; // found, comparisons + } + } + return new int[]{0, comparisons}; // not found, comparisons + } + + // --------------------------------------------------------------- + // FIXED: early exit when entry.start_addr > addr (sorted invariant) + // --------------------------------------------------------------- + + /** + * Models the patched search_checkpoint_list. + * Breaks as soon as entry.start_addr > addr. + */ + static int[] fixed_search(List sortedList, int addr) { + int comparisons = 0; + for (Checkpoint cp : sortedList) { + comparisons++; + if (cp.start_addr > addr) { + break; // sorted list: no further match possible + } + if (addr >= cp.start_addr && addr <= cp.end_addr) { + return new int[]{1, comparisons}; // found + } + } + return new int[]{0, comparisons}; // not found + } + + // --------------------------------------------------------------- + // Build a sorted list of N single-address breakpoints + // --------------------------------------------------------------- + + static List buildSortedBreakpoints(int n) { + List list = new ArrayList<>(); + // Breakpoints at even addresses: 0x0100, 0x0102, ... step 2 + for (int i = 0; i < n; i++) { + int addr = 0x0100 + i * 2; + list.add(new Checkpoint(addr, addr)); + } + return list; + } + + // --------------------------------------------------------------- + // Tests + // --------------------------------------------------------------- + + static int passed = 0; + static int failed = 0; + + static void check(String name, boolean condition) { + if (condition) { + System.out.println("PASS " + name); + passed++; + } else { + System.out.println("FAIL " + name); + failed++; + } + } + + public static void main(String[] args) { + + // --- Correctness --- + + List bps = buildSortedBreakpoints(10); + // Addr 0x0100 is our first breakpoint + int[] r1d = defective_search(bps, 0x0100); + int[] r1f = fixed_search(bps, 0x0100); + check("vice-0001 correctness: found first breakpoint (defective)", r1d[0] == 1); + check("vice-0001 correctness: found first breakpoint (fixed)", r1f[0] == 1); + + // Addr 0x0112 = 0x0100 + 9*2 = last breakpoint + int[] r2d = defective_search(bps, 0x0112); + int[] r2f = fixed_search(bps, 0x0112); + check("vice-0001 correctness: found last breakpoint (defective)", r2d[0] == 1); + check("vice-0001 correctness: found last breakpoint (fixed)", r2f[0] == 1); + + // Addr 0x0050 is below all breakpoints — miss + int[] r3d = defective_search(bps, 0x0050); + int[] r3f = fixed_search(bps, 0x0050); + check("vice-0001 correctness: miss below range (defective)", r3d[0] == 0); + check("vice-0001 correctness: miss below range (fixed)", r3f[0] == 0); + // defective must scan all 10; fixed exits on first entry (start 0x0100 > 0x0050) + check("vice-0001 correctness: fixed exits after 1 comparison on miss below range", + r3f[1] == 1); + check("vice-0001 correctness: defective scans all N on miss below range", + r3d[1] == 10); + + // Addr 0x8000 is above all breakpoints — miss + int[] r4d = defective_search(bps, 0x8000); + int[] r4f = fixed_search(bps, 0x8000); + check("vice-0001 correctness: miss above range (defective)", r4d[0] == 0); + check("vice-0001 correctness: miss above range (fixed)", r4f[0] == 0); + + // --- Performance: measure total comparisons at N=200 breakpoints --- + int N = 200; + List bigBps = buildSortedBreakpoints(N); + + // Simulate M consecutive opcode checks at addresses mostly BELOW the + // first breakpoint (common case: code running without hitting any bp). + int M = 10_000; + long defectiveTotal = 0; + long fixedTotal = 0; + + // Also measure real wall time + long t0 = System.currentTimeMillis(); + for (int i = 0; i < M; i++) { + int[] r = defective_search(bigBps, 0x0050 + (i % 16)); + defectiveTotal += r[1]; + } + long defTime = System.currentTimeMillis() - t0; + + t0 = System.currentTimeMillis(); + for (int i = 0; i < M; i++) { + int[] r = fixed_search(bigBps, 0x0050 + (i % 16)); + fixedTotal += r[1]; + } + long fixTime = System.currentTimeMillis() - t0; + + // defective: N comparisons per miss; fixed: 1 comparison per miss + double ratio = (fixedTotal == 0) ? Double.MAX_VALUE : (double) defectiveTotal / fixedTotal; + System.out.printf( + "vice-0001 performance: N=%d M=%d defect_cmps=%d fixed_cmps=%d ratio=%.1fx%n", + N, M, defectiveTotal, fixedTotal, ratio); + + // Expect at least N/4 speedup for below-range misses + check("vice-0001 performance: fixed comparison count <= defective/N", + fixedTotal * N <= defectiveTotal * 2); + + System.out.printf("%n%d/%d tests passed%n", passed, passed + failed); + if (failed > 0) System.exit(1); + } +}