From b04b3d389f5f8b4c793caf8a3b4af875785b2391 Mon Sep 17 00:00:00 2001 From: Vladimir Dolzhenko Date: Thu, 2 Feb 2023 17:23:51 +0100 Subject: [PATCH] Add ability for DeclarationDescriptor validation Not fully initialized DeclarationDescriptor could leak to bindingTrace. It is hard to detect the outcome of that kind of leakage due to possibility to have not fully initialized instances if PCE happens in the middle of publication and instance full initialization. #KT-56364 #KT-56388 #KT-54085 #KT-55150 --- .../kotlin/resolve/BindingTraceContext.java | 19 ++++++++++++++++--- .../lazy/descriptors/LazyAnnotations.kt | 7 ++++++- .../lazy/descriptors/LazyClassDescriptor.java | 7 +++++++ .../descriptors/DeclarationDescriptor.java | 2 +- .../descriptors/ValidateableDescriptor.java | 10 ++++++++++ .../DeclarationDescriptorNonRootImpl.java | 10 ++++++---- .../impl/FunctionDescriptorImpl.java | 5 +++++ .../impl/PropertyDescriptorImpl.java | 7 ++++++- 8 files changed, 57 insertions(+), 10 deletions(-) create mode 100644 core/descriptors/src/org/jetbrains/kotlin/descriptors/ValidateableDescriptor.java diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/BindingTraceContext.java b/compiler/frontend/src/org/jetbrains/kotlin/resolve/BindingTraceContext.java index 49946047706..c368ddf61e5 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/BindingTraceContext.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/BindingTraceContext.java @@ -17,9 +17,11 @@ package org.jetbrains.kotlin.resolve; import com.google.common.collect.ImmutableMap; +import com.intellij.openapi.progress.ProgressManager; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.TestOnly; +import org.jetbrains.kotlin.descriptors.ValidateableDescriptor; import org.jetbrains.kotlin.diagnostics.Diagnostic; import org.jetbrains.kotlin.psi.KtExpression; import org.jetbrains.kotlin.resolve.diagnostics.BindingContextSuppressCache; @@ -33,6 +35,7 @@ import org.jetbrains.kotlin.util.slicedMap.*; import java.util.Collection; public class BindingTraceContext implements BindingTrace { + private static final boolean VALIDATION = Boolean.parseBoolean(System.getProperty("kotlin.bindingTrace.validation")); // These flags are used for debugging of "Rewrite at slice..." exceptions /* package */ final static boolean TRACK_REWRITES = false; /* package */ final static boolean TRACK_WITH_STACK_TRACES = true; @@ -40,6 +43,8 @@ public class BindingTraceContext implements BindingTrace { private final MutableSlicedMap map; private final MutableDiagnosticsWithSuppression mutableDiagnostics; + private final boolean isValidationEnabled; + private final BindingContext bindingContext = new CleanableBindingContext() { @NotNull @Override @@ -91,20 +96,25 @@ public class BindingTraceContext implements BindingTrace { } public BindingTraceContext(BindingTraceFilter filter, boolean allowSliceRewrite) { - this(TRACK_REWRITES && !allowSliceRewrite ? new TrackingSlicedMap(TRACK_WITH_STACK_TRACES) : new SlicedMapImpl(allowSliceRewrite), filter); + this(filter, allowSliceRewrite, VALIDATION); } - private BindingTraceContext(@NotNull MutableSlicedMap map, BindingTraceFilter filter) { + public BindingTraceContext(BindingTraceFilter filter, boolean allowSliceRewrite, boolean isValidationEnabled) { + this(TRACK_REWRITES && !allowSliceRewrite ? new TrackingSlicedMap(TRACK_WITH_STACK_TRACES) : new SlicedMapImpl(allowSliceRewrite), filter, isValidationEnabled); + } + + private BindingTraceContext(@NotNull MutableSlicedMap map, BindingTraceFilter filter, boolean isValidationEnabled) { this.map = map; this.mutableDiagnostics = filter.getIgnoreDiagnostics() ? null : new MutableDiagnosticsWithSuppression(new BindingContextSuppressCache(bindingContext), Diagnostics.Companion.getEMPTY()); + this.isValidationEnabled = isValidationEnabled; } @TestOnly public static BindingTraceContext createTraceableBindingTrace() { - return new BindingTraceContext(new TrackingSlicedMap(TRACK_WITH_STACK_TRACES), BindingTraceFilter.Companion.getACCEPT_ALL()); + return new BindingTraceContext(new TrackingSlicedMap(TRACK_WITH_STACK_TRACES), BindingTraceFilter.Companion.getACCEPT_ALL(), VALIDATION); } @Override @@ -134,6 +144,9 @@ public class BindingTraceContext implements BindingTrace { @Override public void record(WritableSlice slice, K key, V value) { + if (isValidationEnabled && value instanceof ValidateableDescriptor && !ProgressManager.getInstance().isInNonCancelableSection()) { + ((ValidateableDescriptor) value).validate(); + } map.put(slice, key, value); } diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/descriptors/LazyAnnotations.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/descriptors/LazyAnnotations.kt index 36c9d16b93e..9ac3c74aa49 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/descriptors/LazyAnnotations.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/descriptors/LazyAnnotations.kt @@ -76,7 +76,7 @@ class LazyAnnotations( class LazyAnnotationDescriptor( val c: LazyAnnotationsContext, val annotationEntry: KtAnnotationEntry -) : AnnotationDescriptor, LazyEntity { +) : AnnotationDescriptor, LazyEntity, ValidateableDescriptor { override val type by c.storageManager.createLazyValue( computable = lazy@{ @@ -128,6 +128,11 @@ class LazyAnnotationDescriptor( fun getSourceForArgument(name: Name): SourceElement = valueArgumentsWithSourceInfo[name]?.second ?: SourceElement.NO_SOURCE + + override fun validate() { + checkNotNull(scope) { "scope == null for $this" } + } + override fun forceResolveAllContents() { ForceResolveUtil.forceResolveAllContents(type) allValueArguments diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/descriptors/LazyClassDescriptor.java b/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/descriptors/LazyClassDescriptor.java index 081b9d18bab..bedc8112631 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/descriptors/LazyClassDescriptor.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/lazy/descriptors/LazyClassDescriptor.java @@ -702,6 +702,13 @@ public class LazyClassDescriptor extends ClassDescriptorBase implements ClassDes return false; } + @Override + public void validate() { + if (parameters == null) { + throw new IllegalStateException("parameters == null for " + this); + } + } + @Override public String toString() { // not using DescriptorRenderer to preserve laziness diff --git a/core/descriptors/src/org/jetbrains/kotlin/descriptors/DeclarationDescriptor.java b/core/descriptors/src/org/jetbrains/kotlin/descriptors/DeclarationDescriptor.java index 232e9823868..254f6690826 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/descriptors/DeclarationDescriptor.java +++ b/core/descriptors/src/org/jetbrains/kotlin/descriptors/DeclarationDescriptor.java @@ -20,7 +20,7 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.kotlin.descriptors.annotations.Annotated; -public interface DeclarationDescriptor extends Annotated, Named { +public interface DeclarationDescriptor extends Annotated, Named, ValidateableDescriptor { /** * @return The descriptor that corresponds to the original declaration of this element. * A descriptor can be obtained from its original by substituting type arguments (of the declaring class diff --git a/core/descriptors/src/org/jetbrains/kotlin/descriptors/ValidateableDescriptor.java b/core/descriptors/src/org/jetbrains/kotlin/descriptors/ValidateableDescriptor.java new file mode 100644 index 00000000000..dce4115c396 --- /dev/null +++ b/core/descriptors/src/org/jetbrains/kotlin/descriptors/ValidateableDescriptor.java @@ -0,0 +1,10 @@ +/* + * Copyright 2010-2023 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. + */ + +package org.jetbrains.kotlin.descriptors; + +public interface ValidateableDescriptor { + default void validate() {} +} diff --git a/core/descriptors/src/org/jetbrains/kotlin/descriptors/impl/DeclarationDescriptorNonRootImpl.java b/core/descriptors/src/org/jetbrains/kotlin/descriptors/impl/DeclarationDescriptorNonRootImpl.java index 6b83fd6d018..ae9963f00dc 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/descriptors/impl/DeclarationDescriptorNonRootImpl.java +++ b/core/descriptors/src/org/jetbrains/kotlin/descriptors/impl/DeclarationDescriptorNonRootImpl.java @@ -17,10 +17,7 @@ package org.jetbrains.kotlin.descriptors.impl; import org.jetbrains.annotations.NotNull; -import org.jetbrains.kotlin.descriptors.DeclarationDescriptor; -import org.jetbrains.kotlin.descriptors.DeclarationDescriptorNonRoot; -import org.jetbrains.kotlin.descriptors.DeclarationDescriptorWithSource; -import org.jetbrains.kotlin.descriptors.SourceElement; +import org.jetbrains.kotlin.descriptors.*; import org.jetbrains.kotlin.descriptors.annotations.Annotations; import org.jetbrains.kotlin.name.Name; @@ -64,4 +61,9 @@ public abstract class DeclarationDescriptorNonRootImpl return source; } + @Override + public void validate() { + containingDeclaration.validate(); + } + } diff --git a/core/descriptors/src/org/jetbrains/kotlin/descriptors/impl/FunctionDescriptorImpl.java b/core/descriptors/src/org/jetbrains/kotlin/descriptors/impl/FunctionDescriptorImpl.java index dc2fe0d6b43..40fc70162a2 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/descriptors/impl/FunctionDescriptorImpl.java +++ b/core/descriptors/src/org/jetbrains/kotlin/descriptors/impl/FunctionDescriptorImpl.java @@ -306,6 +306,11 @@ public abstract class FunctionDescriptorImpl extends DeclarationDescriptorNonRoo return parameters; } + @Override + public void validate() { + getTypeParameters(); + } + @Override @NotNull public List getValueParameters() { diff --git a/core/descriptors/src/org/jetbrains/kotlin/descriptors/impl/PropertyDescriptorImpl.java b/core/descriptors/src/org/jetbrains/kotlin/descriptors/impl/PropertyDescriptorImpl.java index f623dc21a27..05d8ca20059 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/descriptors/impl/PropertyDescriptorImpl.java +++ b/core/descriptors/src/org/jetbrains/kotlin/descriptors/impl/PropertyDescriptorImpl.java @@ -180,11 +180,16 @@ public class PropertyDescriptorImpl extends VariableDescriptorWithInitializerImp List parameters = typeParameters; // Diagnostics for EA-212070 if (parameters == null) { - throw new IllegalStateException("typeParameters == null for " + this.toString()); + throw new IllegalStateException("typeParameters == null for " + this); } return parameters; } + @Override + public void validate() { + getTypeParameters(); + } + @Override @NotNull public List getContextReceiverParameters() {