From 299a45ccbf576958e3e00c4e5d73b33df04a0dbc Mon Sep 17 00:00:00 2001 From: Anton Bannykh Date: Fri, 21 Dec 2018 18:58:11 +0300 Subject: [PATCH] JS: validate AST when serializing to IC cache --- ...csTestWithJsStdLibAndBackendCompilation.kt | 2 +- .../kotlin/js/inline/util/collectUtils.kt | 35 ++++++++---------- .../serialization/js/ast/JsAstSerializer.kt | 5 ++- .../js/test/AbstractJsLineNumberTest.kt | 2 +- .../jetbrains/kotlin/js/test/BasicBoxTest.kt | 2 +- .../kotlin/js/facade/K2JSTranslator.kt | 36 ++++++++++++++++--- 6 files changed, 54 insertions(+), 28 deletions(-) diff --git a/compiler/tests-common/tests/org/jetbrains/kotlin/checkers/AbstractDiagnosticsTestWithJsStdLibAndBackendCompilation.kt b/compiler/tests-common/tests/org/jetbrains/kotlin/checkers/AbstractDiagnosticsTestWithJsStdLibAndBackendCompilation.kt index 94f6bb253e0..0aa7babb48c 100644 --- a/compiler/tests-common/tests/org/jetbrains/kotlin/checkers/AbstractDiagnosticsTestWithJsStdLibAndBackendCompilation.kt +++ b/compiler/tests-common/tests/org/jetbrains/kotlin/checkers/AbstractDiagnosticsTestWithJsStdLibAndBackendCompilation.kt @@ -40,7 +40,7 @@ abstract class AbstractDiagnosticsTestWithJsStdLibAndBackendCompilation : Abstra val diagnostics = analysisResult.bindingTrace.bindingContext.diagnostics if (!hasError(diagnostics)) { - val translator = K2JSTranslator(config) + val translator = K2JSTranslator(config, true) translator.translate(object : JsConfig.Reporter() {}, files, MainCallParameters.noCall(), analysisResult) } diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/collectUtils.kt b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/collectUtils.kt index fdced6b7099..0ae35d32bc1 100644 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/collectUtils.kt +++ b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/util/collectUtils.kt @@ -107,6 +107,11 @@ fun collectDefinedNames(scope: JsNode): Set { super.visitExpressionStatement(x) } + override fun visitBreak(x: JsBreak) { + x.label?.name?.let { names += it } + super.visitBreak(x) + } + override fun visitCatch(x: JsCatch) { names += x.parameter.name super.visitCatch(x) @@ -134,6 +139,16 @@ fun collectDefinedNamesInAllScopes(scope: JsNode): Set { x.name?.let { names += it } names += x.parameters.map { it.name } } + + override fun visitBreak(x: JsBreak) { + x.label?.name?.let { names += it } + super.visitBreak(x) + } + + override fun visitCatch(x: JsCatch) { + names += x.parameter.name + super.visitCatch(x) + } }.accept(scope) return names @@ -238,26 +253,6 @@ fun extractFunction(expression: JsExpression) = when (expression) { else -> InlineMetadata.decompose(expression)?.function ?: InlineMetadata.tryExtractFunction(expression) } -fun collectInlineFunctionTags(config: JsConfig, fragments: List): Set { - val result = mutableSetOf() - for (fragment in fragments) { - collectInlineFunctionTags(config, fragment.declarationBlock, result) - collectInlineFunctionTags(config, fragment.initializerBlock, result) - } - return result -} - -private fun collectInlineFunctionTags(config: JsConfig, scope: JsNode, result: MutableSet) { - scope.accept(object : RecursiveJsVisitor() { - override fun visitInvocation(invocation: JsInvocation) { - invocation.descriptor?.let { - result.add(Namer.getFunctionTag(it, config)) - } - super.visitInvocation(invocation) - } - }) -} - fun collectInstances(klass: Class, scope: JsNode): List { return with(InstanceCollector(klass, visitNestedDeclarations = false)) { accept(scope) diff --git a/js/js.serializer/src/org/jetbrains/kotlin/serialization/js/ast/JsAstSerializer.kt b/js/js.serializer/src/org/jetbrains/kotlin/serialization/js/ast/JsAstSerializer.kt index 46d03918357..4f80451a41a 100644 --- a/js/js.serializer/src/org/jetbrains/kotlin/serialization/js/ast/JsAstSerializer.kt +++ b/js/js.serializer/src/org/jetbrains/kotlin/serialization/js/ast/JsAstSerializer.kt @@ -30,7 +30,8 @@ import java.io.OutputStream import java.util.* import org.jetbrains.kotlin.resolve.inline.InlineStrategy as KotlinInlineStrategy -class JsAstSerializer(private val pathResolver: (File) -> String) { +class JsAstSerializer(private val jsAstValidator: ((JsProgramFragment, Set) -> Unit)?, + private val pathResolver: (File) -> String) { private val nameTableBuilder = NameTable.newBuilder() private val stringTableBuilder = StringTable.newBuilder() private val nameMap = mutableMapOf() @@ -123,6 +124,8 @@ class JsAstSerializer(private val pathResolver: (File) -> String) { fragmentBuilder.addInlinedFunctionWrappers(builder.build()) } + jsAstValidator?.let { it(fragment, nameMap.keys) } + return fragmentBuilder.build() } diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/AbstractJsLineNumberTest.kt b/js/js.tests/test/org/jetbrains/kotlin/js/test/AbstractJsLineNumberTest.kt index 979fb6d9e7f..29d3e799d21 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/AbstractJsLineNumberTest.kt +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/AbstractJsLineNumberTest.kt @@ -55,7 +55,7 @@ abstract class AbstractJsLineNumberTest : KotlinTestWithEnvironment() { orderedModules.asReversed().forEach { module -> val baseOutputPath = module.outputFileName(file) - val translator = K2JSTranslator(createConfig(module, file, modules)) + val translator = K2JSTranslator(createConfig(module, file, modules), true) val units = module.files.map { TranslationUnit.SourceFile(createPsiFile(it.fileName)) } val translationResult = translator.translateUnits(ExceptionThrowingReporter, units, MainCallParameters.noCall()) diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/BasicBoxTest.kt b/js/js.tests/test/org/jetbrains/kotlin/js/test/BasicBoxTest.kt index 0c5e3d4c2c6..4dd80ae8f64 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/BasicBoxTest.kt +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/BasicBoxTest.kt @@ -461,7 +461,7 @@ abstract class BasicBoxTest( runtime: JsIrTestRuntime, isMainModule: Boolean ) { - val translator = K2JSTranslator(config) + val translator = K2JSTranslator(config, true) val translationResult = translator.translateUnits(ExceptionThrowingReporter, units, mainCallParameters) if (translationResult !is TranslationResult.Success) { diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/facade/K2JSTranslator.kt b/js/js.translator/src/org/jetbrains/kotlin/js/facade/K2JSTranslator.kt index b793ea6fa68..2d3681edb70 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/facade/K2JSTranslator.kt +++ b/js/js.translator/src/org/jetbrains/kotlin/js/facade/K2JSTranslator.kt @@ -24,10 +24,8 @@ import org.jetbrains.kotlin.js.analyze.TopDownAnalyzerFacadeForJS import org.jetbrains.kotlin.js.analyzer.JsAnalysisResult import org.jetbrains.kotlin.js.config.JSConfigurationKeys import org.jetbrains.kotlin.js.config.JsConfig -import org.jetbrains.kotlin.js.coroutine.CoroutineTransformer import org.jetbrains.kotlin.js.facade.exceptions.TranslationException import org.jetbrains.kotlin.js.inline.JsInliner -import org.jetbrains.kotlin.js.inline.clean.LabeledBlockToDoWhileTransformation import org.jetbrains.kotlin.js.inline.clean.* import org.jetbrains.kotlin.js.sourceMap.SourceFilePathResolver import org.jetbrains.kotlin.js.translate.general.Translation @@ -43,7 +41,12 @@ import java.io.IOException import java.util.ArrayList import org.jetbrains.kotlin.diagnostics.DiagnosticUtils.hasError +import org.jetbrains.kotlin.js.backend.ast.JsBlock +import org.jetbrains.kotlin.js.backend.ast.JsName +import org.jetbrains.kotlin.js.backend.ast.JsProgramFragment +import org.jetbrains.kotlin.js.backend.ast.JsStatement import org.jetbrains.kotlin.js.coroutine.transformCoroutines +import org.jetbrains.kotlin.js.inline.util.collectDefinedNamesInAllScopes import org.jetbrains.kotlin.js.translate.general.AstGenerationResult import org.jetbrains.kotlin.resolve.BindingTrace import org.jetbrains.kotlin.serialization.js.ast.JsAstProtoBuf @@ -51,7 +54,10 @@ import org.jetbrains.kotlin.serialization.js.ast.JsAstProtoBuf /** * An entry point of translator. */ -class K2JSTranslator(private val config: JsConfig) { +class K2JSTranslator @JvmOverloads constructor( + private val config: JsConfig, + private val shouldValidateJsAst: Boolean = false +) { private val incrementalResults = config.configuration.get(JSConfigurationKeys.INCREMENTAL_RESULTS_CONSUMER) @@ -165,7 +171,7 @@ class K2JSTranslator(private val config: JsConfig) { ) { if (incrementalResults == null) return - val serializer = JsAstSerializer { file -> + val serializer = JsAstSerializer(if (shouldValidateJsAst) ::validateJsAst else null) { file -> try { pathResolver.getPathRelativeToSourceRoots(file) } catch (e: IOException) { @@ -205,4 +211,26 @@ class K2JSTranslator(private val config: JsConfig) { headerBuilder.build().writeTo(output) return output.toByteArray() } + + private fun validateJsAst(fragment: JsProgramFragment, serializedNames: Set) { + val knownNames = mutableSetOf().apply { + fragment.nameBindings.mapTo(this) { it.name } + fragment.importedModules.mapTo(this) { it.internalName } + } + + val allCode = JsBlock(mutableListOf().apply { + add(fragment.declarationBlock) + add(fragment.exportBlock) + add(fragment.initializerBlock) + fragment.tests?.let { add(it) } + fragment.mainFunction?.let { add(it) } + addAll(fragment.inlinedFunctionWrappers.values) + }) + + val definedNames = collectDefinedNamesInAllScopes(allCode) + + serializedNames.forEach { + assert(!it.isTemporary || it in definedNames || it in knownNames) { "JsName ${it.ident} is unbound" } + } + } }