diff --git a/defects/graal/patch/graal-0001-registerverifier-worklist-arraylist-contains.md b/defects/graal/patch/graal-0001-registerverifier-worklist-arraylist-contains.md new file mode 100644 index 000000000..b12202cd6 --- /dev/null +++ b/defects/graal/patch/graal-0001-registerverifier-worklist-arraylist-contains.md @@ -0,0 +1,86 @@ +# UNDF: (pending) +# graal-0001: RegisterVerifier.addToWorkList — ArrayList.contains() O(B²) in LSRA verification worklist + +## CWE-407 — Algorithmic Complexity: O(N) ArrayList.contains() per block in register verifier worklist BFS + +| Field | Value | +|--------------|-------| +| ID | graal-0001 | +| Severity | MEDIUM | +| Ecosystem | graal | +| Package | jdk.graal.compiler | +| File | `compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/lir/alloc/lsra/RegisterVerifier.java` | +| Lines | 50, 72–76, 80 | +| Complexity | O(B) per block; O(B²) to process B-block CFG | +| Hot path | Called during LSRA (Linear Scan Register Allocation) verification phase of JIT compilation | + +## Defect + +`RegisterVerifier.addToWorkList` uses `ArrayList>` as the worklist and checks for +duplicates with `workList.contains(block)` — an O(N) linear scan: + +```java +// RegisterVerifier.java:50 +ArrayList> workList; // ArrayList → contains() is O(N) + +// RegisterVerifier.java:72-76 (DEFECT) +void addToWorkList(BasicBlock block) { + if (!workList.contains(block)) { // O(N) linear scan of workList + workList.add(block); + } +} + +// RegisterVerifier.java:78-83 +RegisterVerifier(LinearScan allocator) { + // ... + workList = new ArrayList<>(16); // ArrayList +} +``` + +The `verify()` method (lines 86-102) drives a BFS: `processBlock()` calls `addToWorkList` for each +successor block. For B basic blocks in the CFG with branching factor F: +- `addToWorkList` is called ~B×F times +- Each call does O(worklist_size) scan ≤ O(B) +- Total: O(B² × F) comparisons + +GraalVM's own codebase uses `EconomicSet` (a memory-efficient HashSet implementation) throughout +the compiler for exactly this use case. `ArrayList.contains` is out of place here. + +## Fix + +Replace `ArrayList` with a `java.util.HashSet>` or `EconomicSet>`, +or add a parallel membership set alongside the list (to preserve FIFO ordering): + +```java +// AFTER — O(B) total for B-block CFG +ArrayList> workList; +EconomicSet> workListSet; // ADD: O(1) membership guard + +RegisterVerifier(LinearScan allocator) { + this.allocator = allocator; + workList = new ArrayList<>(16); + workListSet = EconomicSet.create(16); // ADD + // ... +} + +void addToWorkList(BasicBlock block) { + if (workListSet.add(block)) { // O(1): add returns false if already present + workList.add(block); + } +} + +// In verify(), after remove(0), also remove from workListSet: +BasicBlock block = workList.remove(0); +workListSet.remove(block); // ADD: keep mirror in sync +processBlock(block); +``` + +## Speedup + +| Blocks (B) | Before (comparisons) | After (comparisons) | Speedup | +|------------|----------------------|---------------------|---------| +| 50 | ~625 | 50 | 12.5× | +| 100 | ~2,500 | 100 | 25× | +| 500 | ~62,500 | 500 | 125× | + +Growth before: O(B²). Growth after: O(B). diff --git a/defects/graal/patch/graal-0002-classfileconstant-resolvemethod-diamond.md b/defects/graal/patch/graal-0002-classfileconstant-resolvemethod-diamond.md new file mode 100644 index 000000000..315a9d651 --- /dev/null +++ b/defects/graal/patch/graal-0002-classfileconstant-resolvemethod-diamond.md @@ -0,0 +1,91 @@ +# UNDF: (pending) +# graal-0002: ClassfileConstant.resolveMethod/resolveField — O(2^D) diamond re-traversal without visited set + +## CWE-407 — Algorithmic Complexity: O(2^D) recursive interface re-traversal in classfile method/field resolution + +| Field | Value | +|--------------|-------| +| ID | graal-0002 | +| Severity | MEDIUM | +| Ecosystem | graal | +| Package | jdk.graal.compiler | +| File | `compiler/src/jdk.graal.compiler/src/jdk/graal/compiler/replacements/classfile/ClassfileConstant.java` | +| Lines | 281–319 | +| Complexity | O(2^D) on interface diamond hierarchies | +| Hot path | Called during classfile-based bytecode provider method/field resolution for Graal substitutions | + +## Defect + +`ClassfileConstant.resolveMethod` and `resolveField` recursively traverse the type hierarchy to find +a matching method/field. They have NO visited set — on a diamond interface hierarchy, shared ancestor +interfaces are traversed exponentially: + +```java +// ClassfileConstant.java:281-299 (DEFECT) +static ResolvedJavaMethod resolveMethod(ClassfileBytecodeProvider context, ResolvedJavaType c, + String name, String descriptor, boolean isStatic) { + ResolvedJavaMethod method = context.findMethod(c, name, descriptor, isStatic); + if (method != null) { return method; } + + if (!c.isJavaLangObject() && !c.isInterface()) { + method = resolveMethod(context, c.getSuperclass(), name, descriptor, isStatic); + if (method != null) { return method; } + } + for (ResolvedJavaType i : c.getInterfaces()) { + method = resolveMethod(context, i, name, descriptor, isStatic); // DEFECT: no visited guard + if (method != null) { return method; } + } + return null; +} + +// ClassfileConstant.java:301-319 — identical pattern for fields (DEFECT) +static ResolvedJavaField resolveField(ClassfileBytecodeProvider context, ResolvedJavaType c, + String name, String fieldType, boolean isStatic) { + // ... same structure, no visited set +} +``` + +For a diamond (class C implements I1 and I2; both I1 and I2 extend I_base): +- `resolveMethod(C)` → `resolveMethod(I1)` → `resolveMethod(I_base)` → not found +- Back in C: → `resolveMethod(I2)` → `resolveMethod(I_base)` → traversed AGAIN + +At diamond depth D, `I_base` is visited 2^D times when the method is not found. + +## Fix + +Add a `Set visited` parameter with a public entry-point wrapper: + +```java +// AFTER — O(N+E) where N=types, E=hierarchy edges +static ResolvedJavaMethod resolveMethod(ClassfileBytecodeProvider context, ResolvedJavaType c, + String name, String descriptor, boolean isStatic) { + return resolveMethod(context, c, name, descriptor, isStatic, new HashSet<>()); +} + +private static ResolvedJavaMethod resolveMethod(ClassfileBytecodeProvider context, ResolvedJavaType c, + String name, String descriptor, boolean isStatic, Set visited) { + if (!visited.add(c)) { return null; } // skip already-visited types + ResolvedJavaMethod method = context.findMethod(c, name, descriptor, isStatic); + if (method != null) { return method; } + if (!c.isJavaLangObject() && !c.isInterface()) { + method = resolveMethod(context, c.getSuperclass(), name, descriptor, isStatic, visited); + if (method != null) { return method; } + } + for (ResolvedJavaType i : c.getInterfaces()) { + method = resolveMethod(context, i, name, descriptor, isStatic, visited); + if (method != null) { return method; } + } + return null; +} +// Same fix applies to resolveField +``` + +## Speedup + +| Diamond depth (D) | Before (visits) | After (visits) | Speedup | +|------------------|----------------|----------------|---------| +| 5 | 31 | 5 | 6× | +| 10 | 1,023 | 10 | 102× | +| 15 | 32,767 | 15 | 2,184× | + +Growth before: O(2^D). Growth after: O(D).