graal-0001/0002: RegisterVerifier workList O(B²) + resolveMethod diamond O(2^D); count 661→663
This commit is contained in:
parent
52328b5720
commit
b5384cea54
2 changed files with 177 additions and 0 deletions
|
|
@ -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<BasicBlock<?>>` as the worklist and checks for
|
||||
duplicates with `workList.contains(block)` — an O(N) linear scan:
|
||||
|
||||
```java
|
||||
// RegisterVerifier.java:50
|
||||
ArrayList<BasicBlock<?>> 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<BasicBlock<?>>` or `EconomicSet<BasicBlock<?>>`,
|
||||
or add a parallel membership set alongside the list (to preserve FIFO ordering):
|
||||
|
||||
```java
|
||||
// AFTER — O(B) total for B-block CFG
|
||||
ArrayList<BasicBlock<?>> workList;
|
||||
EconomicSet<BasicBlock<?>> 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).
|
||||
|
|
@ -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<ResolvedJavaType> 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<ResolvedJavaType> 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).
|
||||
Loading…
Add table
Add a link
Reference in a new issue