# HG changeset patch # User attila # Date 1441886427 -7200 # Node ID e7d479909bc57ff06761b3a0383b5e0b174a320d # Parent ca0e29811b81ca380ef2b0e8481b1410f924e46a 8135262: Sanitize CodeInstaller API Reviewed-by: hannesw, sundar diff -r ca0e29811b81 -r e7d479909bc5 src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CompilationPhase.java --- a/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CompilationPhase.java Thu Sep 10 13:50:04 2015 +0200 +++ b/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CompilationPhase.java Thu Sep 10 14:00:27 2015 +0200 @@ -591,8 +591,8 @@ Class rootClass = null; long length = 0L; - final CodeInstaller codeInstaller = compiler.getCodeInstaller(); - final Map bytecode = compiler.getBytecode(); + final CodeInstaller codeInstaller = compiler.getCodeInstaller(); + final Map bytecode = compiler.getBytecode(); for (final Entry entry : bytecode.entrySet()) { final String className = entry.getKey(); diff -r ca0e29811b81 -r e7d479909bc5 src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/Compiler.java --- a/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/Compiler.java Thu Sep 10 13:50:04 2015 +0200 +++ b/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/Compiler.java Thu Sep 10 14:00:27 2015 +0200 @@ -101,7 +101,7 @@ private final ConstantData constantData; - private final CodeInstaller installer; + private final CodeInstaller installer; /** logger for compiler, trampolines and related code generation events * that affect classes */ @@ -352,47 +352,83 @@ private static final AtomicInteger COMPILATION_ID = new AtomicInteger(0); /** - * Constructor + * Creates a new compiler instance for initial compilation of a script. * - * @param context context - * @param env script environment * @param installer code installer * @param source source to compile * @param errors error manager * @param isStrict is this a strict compilation + * @return a new compiler */ - public Compiler( - final Context context, - final ScriptEnvironment env, - final CodeInstaller installer, + public static Compiler forInitialCompilation( + final CodeInstaller installer, final Source source, final ErrorManager errors, final boolean isStrict) { - this(context, env, installer, source, errors, isStrict, false, null, null, null, null, null, null); + return new Compiler(installer.getContext(), installer, source, errors, isStrict); } /** - * Constructor + * Creates a compiler without a code installer. It can only be used to compile code, not install the + * generated classes and as such it is useful only for implementation of {@code --compile-only} command + * line option. + * @param context the current context + * @param source source to compile + * @param isStrict is this a strict compilation + * @return a new compiler + */ + public static Compiler forNoInstallerCompilation( + final Context context, + final Source source, + final boolean isStrict) { + return new Compiler(context, null, source, context.getErrorManager(), isStrict); + } + + /** + * Creates a compiler for an on-demand compilation job. * - * @param context context - * @param env script environment * @param installer code installer * @param source source to compile - * @param errors error manager * @param isStrict is this a strict compilation - * @param isOnDemand is this an on demand compilation * @param compiledFunction compiled function, if any * @param types parameter and return value type information, if any is known * @param invalidatedProgramPoints invalidated program points for recompilation * @param typeInformationFile descriptor of the location where type information is persisted * @param continuationEntryPoints continuation entry points for restof method * @param runtimeScope runtime scope for recompilation type lookup in {@code TypeEvaluator} + * @return a new compiler */ - @SuppressWarnings("unused") - public Compiler( + public static Compiler forOnDemandCompilation( + final CodeInstaller installer, + final Source source, + final boolean isStrict, + final RecompilableScriptFunctionData compiledFunction, + final TypeMap types, + final Map invalidatedProgramPoints, + final Object typeInformationFile, + final int[] continuationEntryPoints, + final ScriptObject runtimeScope) { + final Context context = installer.getContext(); + return new Compiler(context, installer, source, context.getErrorManager(), isStrict, true, + compiledFunction, types, invalidatedProgramPoints, typeInformationFile, + continuationEntryPoints, runtimeScope); + } + + /** + * Convenience constructor for non on-demand compiler instances. + */ + private Compiler( final Context context, - final ScriptEnvironment env, - final CodeInstaller installer, + final CodeInstaller installer, + final Source source, + final ErrorManager errors, + final boolean isStrict) { + this(context, installer, source, errors, isStrict, false, null, null, null, null, null, null); + } + + private Compiler( + final Context context, + final CodeInstaller installer, final Source source, final ErrorManager errors, final boolean isStrict, @@ -404,7 +440,7 @@ final int[] continuationEntryPoints, final ScriptObject runtimeScope) { this.context = context; - this.env = env; + this.env = context.getEnv(); this.installer = installer; this.constantData = new ConstantData(); this.compileUnits = CompileUnit.createCompileUnitSet(); @@ -416,7 +452,7 @@ this.onDemand = isOnDemand; this.compiledFunction = compiledFunction; this.types = types; - this.invalidatedProgramPoints = invalidatedProgramPoints == null ? new HashMap() : invalidatedProgramPoints; + this.invalidatedProgramPoints = invalidatedProgramPoints == null ? new HashMap<>() : invalidatedProgramPoints; this.typeInformationFile = typeInformationFile; this.continuationEntryPoints = continuationEntryPoints == null ? null: continuationEntryPoints.clone(); this.typeEvaluator = new TypeEvaluator(this, runtimeScope); @@ -426,7 +462,7 @@ this.optimistic = env._optimistic_types; } - private static String safeSourceName(final ScriptEnvironment env, final CodeInstaller installer, final Source source) { + private String safeSourceName() { String baseName = new File(source.getName()).getName(); final int index = baseName.lastIndexOf(".js"); @@ -485,7 +521,7 @@ sb.append('$'); } - sb.append(Compiler.safeSourceName(env, installer, source)); + sb.append(safeSourceName()); return sb.toString(); } @@ -684,7 +720,7 @@ return constantData; } - CodeInstaller getCodeInstaller() { + CodeInstaller getCodeInstaller() { return installer; } diff -r ca0e29811b81 -r e7d479909bc5 src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/CodeInstaller.java --- a/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/CodeInstaller.java Thu Sep 10 13:50:04 2015 +0200 +++ b/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/CodeInstaller.java Thu Sep 10 14:00:27 2015 +0200 @@ -38,15 +38,14 @@ * The compiler still retains most of the state around code emission * and management internally, so this is to avoid passing around any * logic that isn't directly related to installing a class - * @param owner class type for this code installer * */ -public interface CodeInstaller { +public interface CodeInstaller { /** - * Return the owner for the CodeInstaller, e.g. a {@link Context} - * @return owner + * Return the {@link Context} associated with this code installer. + * @return the context. */ - public T getOwner(); + public Context getContext(); /** * Install a class. @@ -106,7 +105,7 @@ * new, independent class loader. * @return a new code installer with a new independent class loader. */ - public CodeInstaller withNewLoader(); + public CodeInstaller withNewLoader(); /** * Returns true if this code installer is compatible with the other code installer. Compatibility is expected to be @@ -115,6 +114,6 @@ * @param other the other code installer tested for compatibility with this code installer. * @return true if this code installer is compatible with the other code installer. */ - public boolean isCompatibleWith(CodeInstaller other); + public boolean isCompatibleWith(CodeInstaller other); } diff -r ca0e29811b81 -r e7d479909bc5 src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/Context.java --- a/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/Context.java Thu Sep 10 13:50:04 2015 +0200 +++ b/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/Context.java Thu Sep 10 14:00:27 2015 +0200 @@ -167,7 +167,7 @@ * ContextCodeInstaller that has the privilege of installing classes in the Context. * Can only be instantiated from inside the context and is opaque to other classes */ - public static class ContextCodeInstaller implements CodeInstaller { + public static class ContextCodeInstaller implements CodeInstaller { private final Context context; private final ScriptLoader loader; private final CodeSource codeSource; @@ -185,13 +185,9 @@ this.codeSource = codeSource; } - /** - * Return the script environment for this installer - * @return ScriptEnvironment - */ @Override - public ScriptEnvironment getOwner() { - return context.env; + public Context getContext() { + return context; } @Override @@ -254,7 +250,7 @@ } @Override - public CodeInstaller withNewLoader() { + public CodeInstaller withNewLoader() { // Reuse this installer if we're within our limits. if (usageCount < MAX_USAGES && bytesDefined < MAX_BYTES_DEFINED) { return this; @@ -263,7 +259,7 @@ } @Override - public boolean isCompatibleWith(final CodeInstaller other) { + public boolean isCompatibleWith(final CodeInstaller other) { if (other instanceof ContextCodeInstaller) { final ContextCodeInstaller cci = (ContextCodeInstaller)other; return cci.context == context && cci.codeSource == codeSource; @@ -1300,14 +1296,12 @@ final URL url = source.getURL(); final ScriptLoader loader = env._loader_per_compile ? createNewLoader() : scriptLoader; final CodeSource cs = new CodeSource(url, (CodeSigner[])null); - final CodeInstaller installer = new ContextCodeInstaller(this, loader, cs); + final CodeInstaller installer = new ContextCodeInstaller(this, loader, cs); if (storedScript == null) { final CompilationPhases phases = Compiler.CompilationPhases.COMPILE_ALL; - final Compiler compiler = new Compiler( - this, - env, + final Compiler compiler = Compiler.forInitialCompilation( installer, source, errMan, diff -r ca0e29811b81 -r e7d479909bc5 src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java --- a/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java Thu Sep 10 13:50:04 2015 +0200 +++ b/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java Thu Sep 10 14:00:27 2015 +0200 @@ -119,7 +119,7 @@ private final Object endParserState; /** Code installer used for all further recompilation/specialization of this ScriptFunction */ - private transient CodeInstaller installer; + private transient CodeInstaller installer; private final Map nestedFunctions; @@ -153,7 +153,7 @@ */ public RecompilableScriptFunctionData( final FunctionNode functionNode, - final CodeInstaller installer, + final CodeInstaller installer, final AllocationStrategy allocationStrategy, final Map nestedFunctions, final Map externalScopeDepths, @@ -285,7 +285,7 @@ * @param src source * @param inst code installer */ - public void initTransients(final Source src, final CodeInstaller inst) { + public void initTransients(final Source src, final CodeInstaller inst) { if (this.source == null && this.installer == null) { this.source = src; this.installer = inst; @@ -500,7 +500,7 @@ } private FunctionNode deserialize(final byte[] serializedAst) { - final ScriptEnvironment env = installer.getOwner(); + final ScriptEnvironment env = installer.getContext().getEnv(); final Timing timing = env._timing; final long t1 = System.nanoTime(); try { @@ -647,8 +647,8 @@ * a new class loader with optimistic typing so that deoptimized code can get reclaimed by GC. * @return a code installer for installing new code. */ - private CodeInstaller getInstallerForNewCode() { - final ScriptEnvironment env = installer.getOwner(); + private CodeInstaller getInstallerForNewCode() { + final ScriptEnvironment env = installer.getContext().getEnv(); return env._optimistic_types || env._loader_per_compile ? installer.withNewLoader() : installer; } @@ -658,15 +658,10 @@ final TypeMap typeMap = typeMap(actualCallSiteType); final Type[] paramTypes = typeMap == null ? null : typeMap.getParameterTypes(functionNodeId); final Object typeInformationFile = OptimisticTypesPersistence.getLocationDescriptor(source, functionNodeId, paramTypes); - final Context context = Context.getContextTrusted(); - return new Compiler( - context, - context.getEnv(), + return Compiler.forOnDemandCompilation( getInstallerForNewCode(), functionNode.getSource(), // source - context.getErrorManager(), isStrict() | functionNode.isStrict(), // is strict - true, // is on demand this, // compiledFunction, i.e. this RecompilableScriptFunctionData typeMap, // type map getEffectiveInvalidatedProgramPoints(invalidatedProgramPoints, typeInformationFile), // invalidated program points @@ -709,7 +704,7 @@ final TypeMap typeMap = typeMap(actualCallSiteType); final Type[] paramTypes = typeMap == null ? null : typeMap.getParameterTypes(functionNodeId); cacheKey = CodeStore.getCacheKey(functionNodeId, paramTypes); - final CodeInstaller newInstaller = getInstallerForNewCode(); + final CodeInstaller newInstaller = getInstallerForNewCode(); final StoredScript script = newInstaller.loadScript(source, cacheKey); if (script != null) { @@ -730,7 +725,7 @@ } boolean usePersistentCodeCache() { - return installer != null && installer.getOwner()._persistent_cache; + return installer != null && installer.getContext().getEnv()._persistent_cache; } private MethodType explicitParams(final MethodType callSiteType) { diff -r ca0e29811b81 -r e7d479909bc5 src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/StoredScript.java --- a/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/StoredScript.java Thu Sep 10 13:50:04 2015 +0200 +++ b/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/StoredScript.java Thu Sep 10 14:00:27 2015 +0200 @@ -77,7 +77,7 @@ return compilationId; } - private Map> installClasses(final Source source, final CodeInstaller installer) { + private Map> installClasses(final Source source, final CodeInstaller installer) { final Map> installedClasses = new HashMap<>(); final byte[] mainClassBytes = classBytes.get(mainClassName); final Class mainClass = installer.install(mainClassName, mainClassBytes); @@ -96,7 +96,7 @@ return installedClasses; } - FunctionInitializer installFunction(final RecompilableScriptFunctionData data, final CodeInstaller installer) { + FunctionInitializer installFunction(final RecompilableScriptFunctionData data, final CodeInstaller installer) { final Map> installedClasses = installClasses(data.getSource(), installer); assert initializers != null; @@ -124,7 +124,7 @@ * @param installer the installer * @return main script class */ - Class installScript(final Source source, final CodeInstaller installer) { + Class installScript(final Source source, final CodeInstaller installer) { final Map> installedClasses = installClasses(source, installer); diff -r ca0e29811b81 -r e7d479909bc5 src/jdk.scripting.nashorn/share/classes/jdk/nashorn/tools/Shell.java --- a/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/tools/Shell.java Thu Sep 10 13:50:04 2015 +0200 +++ b/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/tools/Shell.java Thu Sep 10 14:00:27 2015 +0200 @@ -42,8 +42,8 @@ import jdk.nashorn.api.scripting.NashornException; import jdk.nashorn.internal.codegen.Compiler; import jdk.nashorn.internal.codegen.Compiler.CompilationPhases; +import jdk.nashorn.internal.ir.Expression; import jdk.nashorn.internal.ir.FunctionNode; -import jdk.nashorn.internal.ir.Expression; import jdk.nashorn.internal.ir.debug.ASTWriter; import jdk.nashorn.internal.ir.debug.PrintVisitor; import jdk.nashorn.internal.objects.Global; @@ -255,12 +255,9 @@ return COMPILATION_ERROR; } - new Compiler( + Compiler.forNoInstallerCompilation( context, - env, - null, //null - pass no code installer - this is compile only functionNode.getSource(), - context.getErrorManager(), env._strict | functionNode.isStrict()). compile(functionNode, CompilationPhases.COMPILE_ALL_NO_INSTALL); diff -r ca0e29811b81 -r e7d479909bc5 test/script/trusted/JDK-8006529.js --- a/test/script/trusted/JDK-8006529.js Thu Sep 10 13:50:04 2015 +0200 +++ b/test/script/trusted/JDK-8006529.js Thu Sep 10 14:00:27 2015 +0200 @@ -120,7 +120,7 @@ var sourceForMethod = Source.class.getMethod("sourceFor", java.lang.String.class, java.lang.String.class) var ParserConstructor = Parser.class.getConstructor(ScriptEnvironment.class, Source.class, ErrorManager.class) -var CompilerConstructor = Compiler.class.getConstructor(Context.class, ScriptEnvironment.class, CodeInstaller.class, Source.class, ErrorManager.class, boolean.class); +var CompilerConstructor = Compiler.class.getMethod("createNoInstallerCompiler", Context.class, Source.class, boolean.class); // compile(script) -- compiles a script specified as a string with its // source code, returns a jdk.nashorn.internal.ir.FunctionNode object @@ -134,7 +134,7 @@ var parser = ParserConstructor.newInstance(env, source, ThrowErrorManager.class.newInstance()); var func = parseMethod.invoke(parser); - var compiler = CompilerConstructor.newInstance(ctxt, env, null, source, null, false); + var compiler = CompilerConstructor.invoke(null, ctxt, source, false); return compileMethod.invoke(compiler, func, phases); };