From 1592f1682c69cca76d85bec39fb6739ba3a34dc0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Kubitz?= Date: Tue, 21 Jan 2025 08:49:16 +0100 Subject: [PATCH] Remove custom ReferenceBinding.hashCode() To avoid possible hash collisions. Instead of manipulating the existing key in a map remove exiting entry and put the updated entry. Use a HashMap instead of SimpleLookupTable. https://github.com/eclipse-jdt/eclipse.jdt.core/issues/3412 --- .../compiler/lookup/ReferenceBinding.java | 9 ---- .../internal/compiler/lookup/TypeSystem.java | 44 ++++++++++++------- 2 files changed, 29 insertions(+), 24 deletions(-) diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ReferenceBinding.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ReferenceBinding.java index 0ae383e631c..6efa9796062 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ReferenceBinding.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/ReferenceBinding.java @@ -1202,15 +1202,6 @@ public TypeVariableBinding getTypeVariable(char[] variableName) { return null; } -@Override -public int hashCode() { - // ensure ReferenceBindings hash to the same position as UnresolvedReferenceBindings so they can be replaced without rehashing - // ALL ReferenceBindings are unique when created so equals() is the same as == - return (this.compoundName == null || this.compoundName.length == 0) - ? super.hashCode() - : CharOperation.hashCode(this.compoundName[this.compoundName.length - 1]); -} - /** * Returns true if the two types have an incompatible common supertype, * e.g. {@code List} and {@code List} diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/TypeSystem.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/TypeSystem.java index 7caa80e8d05..bc8386c7322 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/TypeSystem.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/TypeSystem.java @@ -20,10 +20,11 @@ package org.eclipse.jdt.internal.compiler.lookup; import java.util.HashMap; +import java.util.Map; import java.util.function.Consumer; import java.util.function.Supplier; +import org.eclipse.jdt.core.compiler.CharOperation; import org.eclipse.jdt.internal.compiler.ast.ASTNode; -import org.eclipse.jdt.internal.compiler.util.SimpleLookupTable; import org.eclipse.jdt.internal.compiler.util.Util; /* TypeSystem: An abstraction responsible for keeping track of types that undergo "derivation" of some sort and the derived types produced thus. @@ -70,6 +71,7 @@ public class TypeSystem { public final class HashedParameterizedTypes { private final class PTBKey implements HotSwappable { + private Integer hash; protected ReferenceBinding type; // must ensure the type is resolved public TypeBinding[] arguments; private ReferenceBinding enclosingType; @@ -135,11 +137,26 @@ final int hash(TypeBinding b) { if(b instanceof WildcardBinding || b instanceof TypeVariableBinding || b.getClass() == ParameterizedTypeBinding.class) { return System.identityHashCode(b); } + if (b instanceof ReferenceBinding referenceBinding) { + return hashCode(referenceBinding); + } return b.hashCode(); } + + final int hashCode(ReferenceBinding referenceBinding) { + // ensure ReferenceBindings hash to the same position as UnresolvedReferenceBindings so they can be replaced without rehashing + // ALL ReferenceBindings are unique when created so equals() is the same as == + return (referenceBinding.compoundName == null || referenceBinding.compoundName.length == 0) + ? super.hashCode() + : CharOperation.hashCode(referenceBinding.compoundName[referenceBinding.compoundName.length - 1]); + } @Override public int hashCode() { - final int prime=31; + if (this.hash != null) { + // ensure to hash to the same after changing this.type in #swapUnresolved + return this.hash; + } + final int prime = 31; int hashCode = 1 + hash(this.type); if (this.enclosingType != null && this.enclosingType.getClass() == ParameterizedTypeBinding.class) { // Note: this works as in swapUnresolved, a null enclosingType is never replaced by a @@ -149,7 +166,8 @@ public int hashCode() { for (int i = 0, length = this.arguments == null ? 0 : this.arguments.length; i < length; i++) { hashCode = hashCode * prime + hash(this.arguments[i]); } - return hashCode; + this.hash = hashCode; + return this.hash; } } @@ -217,12 +235,12 @@ void put (ReferenceBinding genericType, TypeBinding[] typeArguments, ReferenceBi private int typeid = TypeIds.T_LastWellKnownTypeId; private TypeBinding [][] types; protected HashedParameterizedTypes parameterizedTypes; // auxiliary fast lookup table for parameterized types. - private SimpleLookupTable annotationTypes; // cannot store in types, since AnnotationBinding is not a TypeBinding and we don't want types to operate at Binding level. + private Map annotationTypes; // cannot store in types, since AnnotationBinding is not a TypeBinding and we don't want types to operate at Binding level. LookupEnvironment environment; public TypeSystem(LookupEnvironment environment) { this.environment = environment; - this.annotationTypes = new SimpleLookupTable(16); + this.annotationTypes = new HashMap<>(); this.typeid = TypeIds.T_LastWellKnownTypeId; this.types = new TypeBinding[TypeIds.T_LastWellKnownTypeId * 2][]; this.parameterizedTypes = new HashedParameterizedTypes(); @@ -558,7 +576,7 @@ alternate code paths. Unless care is exercised, we will end up with duplicate ob We may return a resolved annotation when requested for unresolved one, but not vice versa. */ public final AnnotationBinding getAnnotationType(ReferenceBinding annotationType, boolean requiredResolved) { - AnnotationBinding annotation = (AnnotationBinding) this.annotationTypes.get(annotationType); + AnnotationBinding annotation = this.annotationTypes.get(annotationType); if (annotation == null) { if (requiredResolved) annotation = new AnnotationBinding(annotationType, Binding.NO_ELEMENT_VALUE_PAIRS); @@ -587,7 +605,7 @@ public void cleanUp(int typeId) { } public void reset() { - this.annotationTypes = new SimpleLookupTable(16); + this.annotationTypes = new HashMap<>(); this.typeid = TypeIds.T_LastWellKnownTypeId; this.types = new TypeBinding[TypeIds.T_LastWellKnownTypeId * 2][]; this.parameterizedTypes = new HashedParameterizedTypes(); @@ -611,14 +629,10 @@ public void updateCaches(UnresolvedReferenceBinding unresolvedType, ReferenceBin } } } - if (this.annotationTypes.get(unresolvedType) != null) { // update the key - Object[] keys = this.annotationTypes.keyTable; - for (int i = 0, l = keys.length; i < l; i++) { - if (keys[i] == unresolvedType) { - keys[i] = resolvedType; // hashCode is based on compoundName so this works. - break; - } - } + AnnotationBinding removed = this.annotationTypes.remove(unresolvedType); + if (removed != null) { + // update the key + this.annotationTypes.put(resolvedType, removed); } }