eclipse-jdt-0001/0002: Scope typesToVisit O(N²) + TypeHierarchy missing superclass guard; count 659→661
This commit is contained in:
parent
55df582f19
commit
565831bba0
2 changed files with 168 additions and 0 deletions
|
|
@ -0,0 +1,84 @@
|
|||
# UNDF: (pending)
|
||||
# eclipse-jdt-0001: Scope.getCommonSuperType — ArrayList.contains() O(N²) in BFS supertypes collection
|
||||
|
||||
## CWE-407 — Algorithmic Complexity: O(N) ArrayList.contains() per node in BFS type hierarchy traversal
|
||||
|
||||
| Field | Value |
|
||||
|--------------|-------|
|
||||
| ID | eclipse-jdt-0001 |
|
||||
| Severity | HIGH |
|
||||
| Ecosystem | eclipse-jdt |
|
||||
| Package | org.eclipse.jdt.core.compiler.batch |
|
||||
| File | `org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/Scope.java` |
|
||||
| Lines | 4273–4383 |
|
||||
| Complexity | O(N) per node; O(N²) to collect N-node supertype set |
|
||||
| Hot path | Called during type inference and common-supertype computation in the JDT compiler |
|
||||
|
||||
## Defect
|
||||
|
||||
`Scope.getCommonSuperType` performs a BFS over the complete type hierarchy of `firstType` to collect
|
||||
all supertypes into `typesToVisit`. The collection is an `ArrayList<TypeBinding>`, and the dedup
|
||||
guard uses `typesToVisit.contains()` — an O(N) linear scan — at every BFS step:
|
||||
|
||||
```java
|
||||
// Scope.java:4273-4383 (DEFECT)
|
||||
List<TypeBinding> typesToVisit = new ArrayList<>(); // ArrayList → contains() is O(N)
|
||||
// ...
|
||||
typesToVisit.add(firstType);
|
||||
int max = 1;
|
||||
for (int i = 0; i < max; i++) {
|
||||
TypeBinding typeToVisit = typesToVisit.get(i);
|
||||
// ...
|
||||
// Each of these contains() calls is O(i) — grows as BFS expands:
|
||||
if (!typesToVisit.contains(elementType)) { // O(N) scan
|
||||
typesToVisit.add(elementType); max++;
|
||||
}
|
||||
// ...
|
||||
if (!typesToVisit.contains(superType)) { // O(N) scan (×4 in different branches)
|
||||
typesToVisit.add(superType); max++;
|
||||
}
|
||||
// ... (multiple more contains() calls for interfaces and superclass)
|
||||
}
|
||||
```
|
||||
|
||||
For N types in the hierarchy, the BFS loop runs N iterations; each iteration performs one or more
|
||||
`contains()` calls that scan up to i elements. Total: O(N²) comparisons.
|
||||
|
||||
`ReferenceBinding` confirms identity semantics (its comment says "ALL ReferenceBindings are unique
|
||||
when created so equals() is the same as =="), so a `HashSet<TypeBinding>` using the existing
|
||||
`hashCode()` (name-based) provides O(1) contains with correct dedup semantics.
|
||||
|
||||
## Fix
|
||||
|
||||
Add a parallel `Set<TypeBinding>` for O(1) membership testing alongside the `ArrayList` (kept for
|
||||
indexed iteration via `.get(i)`):
|
||||
|
||||
```java
|
||||
// AFTER — O(N) total for N-node BFS
|
||||
List<TypeBinding> typesToVisit = new ArrayList<>();
|
||||
Set<TypeBinding> typesToVisitSet = new HashSet<>(); // ADD: O(1) membership guard
|
||||
|
||||
typesToVisit.add(firstType);
|
||||
typesToVisitSet.add(firstType); // ADD
|
||||
int max = 1;
|
||||
for (int i = 0; i < max; i++) {
|
||||
TypeBinding typeToVisit = typesToVisit.get(i);
|
||||
// ...
|
||||
// Replace all: if (!typesToVisit.contains(x)) { typesToVisit.add(x); max++; }
|
||||
// With: if (typesToVisitSet.add(x)) { typesToVisit.add(x); max++; }
|
||||
if (typesToVisitSet.add(elementType)) { // O(1) HashSet.add returns false if present
|
||||
typesToVisit.add(elementType); max++;
|
||||
}
|
||||
// ... (apply same replacement to all ~7 contains() checks)
|
||||
}
|
||||
```
|
||||
|
||||
## Speedup
|
||||
|
||||
| Types in hierarchy (N) | Before (comparisons) | After (comparisons) | Speedup |
|
||||
|------------------------|----------------------|---------------------|---------|
|
||||
| 20 | ~200 | 20 | 10× |
|
||||
| 50 | ~1,250 | 50 | 25× |
|
||||
| 100 | ~5,000 | 100 | 50× |
|
||||
|
||||
Growth before: O(N²). Growth after: O(N).
|
||||
|
|
@ -0,0 +1,84 @@
|
|||
# UNDF: (pending)
|
||||
# eclipse-jdt-0002: TypeHierarchy.getAllSupertypes0 — missing Set.add() guard on superclass recursion
|
||||
|
||||
## CWE-407 — Algorithmic Complexity: Unconditional recursion into already-visited superclass
|
||||
|
||||
| Field | Value |
|
||||
|--------------|-------|
|
||||
| ID | eclipse-jdt-0002 |
|
||||
| Severity | MEDIUM |
|
||||
| Ecosystem | eclipse-jdt |
|
||||
| Package | org.eclipse.jdt.core |
|
||||
| File | `org.eclipse.jdt.core/model/org/eclipse/jdt/internal/core/hierarchy/TypeHierarchy.java` |
|
||||
| Lines | 483–498 |
|
||||
| Complexity | Unnecessary superclass subtree re-traversal when superclass already visited via interface path |
|
||||
| Hot path | Called during type hierarchy computation for IDE features (Open Type Hierarchy, completion, etc.) |
|
||||
|
||||
## Defect
|
||||
|
||||
`TypeHierarchy.getAllSupertypes0` recurses through both superinterfaces AND the superclass chain.
|
||||
For superinterfaces it correctly guards with `if (supers.add(superinterface))` — if the interface
|
||||
is already in the set, recursion is skipped. But for the superclass, `supers.add(superclass)` is
|
||||
called without checking the return value:
|
||||
|
||||
```java
|
||||
// TypeHierarchy.java:483-498 (DEFECT)
|
||||
private Set<IType> getAllSupertypes0(IType type, Set<IType> supers) {
|
||||
IType[] superinterfaces = this.typeToSuperInterfaces.get(type);
|
||||
if (superinterfaces == null) {
|
||||
return supers;
|
||||
}
|
||||
for (IType superinterface : superinterfaces) {
|
||||
if (supers.add(superinterface)) { // CORRECT: guard on interface recursion
|
||||
supers = getAllSuperInterfaces0(superinterface, supers);
|
||||
}
|
||||
}
|
||||
IType superclass = this.classToSuperclass.get(type);
|
||||
if (superclass != null) {
|
||||
supers.add(superclass); // DEFECT: ignores return value
|
||||
supers = getAllSupertypes0(superclass, supers); // recurses unconditionally
|
||||
}
|
||||
return supers;
|
||||
}
|
||||
```
|
||||
|
||||
When a class's superclass was already added to `supers` via an interface path (e.g., in a hierarchy
|
||||
where a mixin interface records the concrete class as its supertype in the model), this method recurses
|
||||
into `getAllSupertypes0(superclass, supers)` even though `superclass` is already fully traversed.
|
||||
The redundant traversal re-processes superclass's interfaces and its own superclass chain before the
|
||||
`supers.add()` guards short-circuit each branch.
|
||||
|
||||
Contrast with the CORRECT guard pattern used for interfaces on line 489.
|
||||
|
||||
## Fix
|
||||
|
||||
Apply the same guard pattern to the superclass recursion:
|
||||
|
||||
```java
|
||||
// AFTER — O(N+E) where N=types, E=hierarchy edges
|
||||
private Set<IType> getAllSupertypes0(IType type, Set<IType> supers) {
|
||||
IType[] superinterfaces = this.typeToSuperInterfaces.get(type);
|
||||
if (superinterfaces == null) {
|
||||
return supers;
|
||||
}
|
||||
for (IType superinterface : superinterfaces) {
|
||||
if (supers.add(superinterface)) {
|
||||
supers = getAllSuperInterfaces0(superinterface, supers);
|
||||
}
|
||||
}
|
||||
IType superclass = this.classToSuperclass.get(type);
|
||||
if (superclass != null) {
|
||||
if (supers.add(superclass)) { // FIX: guard matches interface pattern
|
||||
supers = getAllSupertypes0(superclass, supers);
|
||||
}
|
||||
}
|
||||
return supers;
|
||||
}
|
||||
```
|
||||
|
||||
## Speedup
|
||||
|
||||
The guard eliminates re-traversal of already-visited superclass subtrees. In deep hierarchies where
|
||||
a superclass appears via both a class path and an interface model path, the entire subtree below
|
||||
the superclass is traversed redundantly without the guard. With the guard, each type is traversed
|
||||
at most once: O(N+E) instead of O((N+E) × revisit_count).
|
||||
Loading…
Add table
Add a link
Reference in a new issue