vice: 1 CWE-407 defect, MOAD 0002-0005 CLEAN
vice-0001: monitor breakpoint list O(N) scan per opcode — search_checkpoint_list traverses sorted checkpoint linked list without early exit. Code comment promises "we can drop out early" but never implements it. Fix: break when entry.start_addr exceeds target address. 200x fewer comparisons for addresses below first breakpoint. 11/11 PASS. DOSBox-X: dosbox-x-0001/0002 previously committed (2026-03-31). MOAD 0002-0005 CLEAN.
This commit is contained in:
parent
4170c71414
commit
24524ac502
4 changed files with 220 additions and 0 deletions
|
|
@ -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.
|
||||
Binary file not shown.
BIN
defects/vice-0001/test/ViceMonitorBreakpointTest.class
Normal file
BIN
defects/vice-0001/test/ViceMonitorBreakpointTest.class
Normal file
Binary file not shown.
171
defects/vice-0001/test/ViceMonitorBreakpointTest.java
Normal file
171
defects/vice-0001/test/ViceMonitorBreakpointTest.java
Normal file
|
|
@ -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<Checkpoint> 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<Checkpoint> 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<Checkpoint> buildSortedBreakpoints(int n) {
|
||||
List<Checkpoint> 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<Checkpoint> 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<Checkpoint> 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);
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue