8135262: Sanitize CodeInstaller API

Reviewed-by: hannesw, sundar

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CompilationPhase.java

@@ -591,7 +591,7 @@ enum CompilationPhase {
             Class<?> rootClass = null;
             long length = 0L;
 
-            final CodeInstaller<ScriptEnvironment> codeInstaller = compiler.getCodeInstaller();
+            final CodeInstaller       codeInstaller = compiler.getCodeInstaller();
             final Map<String, byte[]> bytecode      = compiler.getBytecode();
 
             for (final Entry<String, byte[]> entry : bytecode.entrySet()) {

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/Compiler.java

@@ -101,7 +101,7 @@ public final class Compiler implements Loggable {
 
     private final ConstantData constantData;
 
-    private final CodeInstaller<ScriptEnvironment> installer;
+    private final CodeInstaller installer;
 
     /** logger for compiler, trampolines and related code generation events
      *  that affect classes */
@@ -352,47 +352,83 @@ public final class Compiler implements Loggable {
     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(
+    public static Compiler forInitialCompilation(
-            final Context context,
+            final CodeInstaller installer,
-            final ScriptEnvironment env,
-            final CodeInstaller<ScriptEnvironment> 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 static Compiler forOnDemandCompilation(
-    public Compiler(
+            final CodeInstaller installer,
+            final Source source,
+            final boolean isStrict,
+            final RecompilableScriptFunctionData compiledFunction,
+            final TypeMap types,
+            final Map<Integer, Type> 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<ScriptEnvironment> 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 @@ public final class Compiler implements Loggable {
             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 @@ public final class Compiler implements Loggable {
         this.onDemand                 = isOnDemand;
         this.compiledFunction         = compiledFunction;
         this.types                    = types;
-        this.invalidatedProgramPoints = invalidatedProgramPoints == null ? new HashMap<Integer, Type>() : 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 @@ public final class Compiler implements Loggable {
         this.optimistic = env._optimistic_types;
     }
 
-    private static String safeSourceName(final ScriptEnvironment env, final CodeInstaller<ScriptEnvironment> installer, final Source source) {
+    private String safeSourceName() {
         String baseName = new File(source.getName()).getName();
 
         final int index = baseName.lastIndexOf(".js");
@@ -485,7 +521,7 @@ public final class Compiler implements Loggable {
             sb.append('$');
         }
 
-        sb.append(Compiler.safeSourceName(env, installer, source));
+        sb.append(safeSourceName());
 
         return sb.toString();
     }
@@ -684,7 +720,7 @@ public final class Compiler implements Loggable {
         return constantData;
     }
 
-    CodeInstaller<ScriptEnvironment> getCodeInstaller() {
+    CodeInstaller getCodeInstaller() {
         return installer;
     }
 

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/CodeInstaller.java

@@ -38,15 +38,14 @@ import jdk.nashorn.internal.codegen.ClassEmitter;
  * 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 <T> owner class type for this code installer
  *
  */
-public interface CodeInstaller<T> {
+public interface CodeInstaller {
     /**
-     * Return the owner for the CodeInstaller, e.g. a {@link Context}
+     * Return the {@link Context} associated with this code installer.
-     * @return owner
+     * @return the context.
      */
-    public T getOwner();
+    public Context getContext();
 
     /**
      * Install a class.
@@ -106,7 +105,7 @@ public interface CodeInstaller<T> {
      * new, independent class loader.
      * @return a new code installer with a new independent class loader.
      */
-    public CodeInstaller<T> 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 @@ public interface CodeInstaller<T> {
      * @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<T> other);
+    public boolean isCompatibleWith(CodeInstaller other);
 
 }

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/Context.java

@@ -167,7 +167,7 @@ public final class Context {
      * 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<ScriptEnvironment> {
+    public static class ContextCodeInstaller implements CodeInstaller {
         private final Context      context;
         private final ScriptLoader loader;
         private final CodeSource   codeSource;
@@ -185,13 +185,9 @@ public final class Context {
             this.codeSource = codeSource;
         }
 
-        /**
-         * Return the script environment for this installer
-         * @return ScriptEnvironment
-         */
         @Override
-        public ScriptEnvironment getOwner() {
+        public Context getContext() {
-            return context.env;
+            return context;
         }
 
         @Override
@@ -254,7 +250,7 @@ public final class Context {
         }
 
         @Override
-        public CodeInstaller<ScriptEnvironment> 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 @@ public final class Context {
         }
 
         @Override
-        public boolean isCompatibleWith(final CodeInstaller<ScriptEnvironment> 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 @@ public final class Context {
         final URL          url    = source.getURL();
         final ScriptLoader loader = env._loader_per_compile ? createNewLoader() : scriptLoader;
         final CodeSource   cs     = new CodeSource(url, (CodeSigner[])null);
-        final CodeInstaller<ScriptEnvironment> 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(
+            final Compiler compiler = Compiler.forInitialCompilation(
-                    this,
-                    env,
                     installer,
                     source,
                     errMan,

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/RecompilableScriptFunctionData.java

@@ -119,7 +119,7 @@ public final class RecompilableScriptFunctionData extends ScriptFunctionData imp
     private final Object endParserState;
 
     /** Code installer used for all further recompilation/specialization of this ScriptFunction */
-    private transient CodeInstaller<ScriptEnvironment> installer;
+    private transient CodeInstaller installer;
 
     private final Map<Integer, RecompilableScriptFunctionData> nestedFunctions;
 
@@ -153,7 +153,7 @@ public final class RecompilableScriptFunctionData extends ScriptFunctionData imp
      */
     public RecompilableScriptFunctionData(
         final FunctionNode functionNode,
-        final CodeInstaller<ScriptEnvironment> installer,
+        final CodeInstaller installer,
         final AllocationStrategy allocationStrategy,
         final Map<Integer, RecompilableScriptFunctionData> nestedFunctions,
         final Map<String, Integer> externalScopeDepths,
@@ -285,7 +285,7 @@ public final class RecompilableScriptFunctionData extends ScriptFunctionData imp
      * @param src source
      * @param inst code installer
      */
-    public void initTransients(final Source src, final CodeInstaller<ScriptEnvironment> 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 @@ public final class RecompilableScriptFunctionData extends ScriptFunctionData imp
     }
 
     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 @@ public final class RecompilableScriptFunctionData extends ScriptFunctionData imp
      * 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<ScriptEnvironment> getInstallerForNewCode() {
+    private CodeInstaller getInstallerForNewCode() {
-        final ScriptEnvironment env = installer.getOwner();
+        final ScriptEnvironment env = installer.getContext().getEnv();
         return env._optimistic_types || env._loader_per_compile ? installer.withNewLoader() : installer;
     }
 
@@ -658,15 +658,10 @@ public final class RecompilableScriptFunctionData extends ScriptFunctionData imp
         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 Compiler.forOnDemandCompilation(
-        return new Compiler(
-                context,
-                context.getEnv(),
                 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 @@ public final class RecompilableScriptFunctionData extends ScriptFunctionData imp
             final TypeMap typeMap = typeMap(actualCallSiteType);
             final Type[] paramTypes = typeMap == null ? null : typeMap.getParameterTypes(functionNodeId);
             cacheKey = CodeStore.getCacheKey(functionNodeId, paramTypes);
-            final CodeInstaller<ScriptEnvironment> newInstaller = getInstallerForNewCode();
+            final CodeInstaller newInstaller = getInstallerForNewCode();
             final StoredScript script = newInstaller.loadScript(source, cacheKey);
 
             if (script != null) {
@@ -730,7 +725,7 @@ public final class RecompilableScriptFunctionData extends ScriptFunctionData imp
     }
 
     boolean usePersistentCodeCache() {
-        return installer != null && installer.getOwner()._persistent_cache;
+        return installer != null && installer.getContext().getEnv()._persistent_cache;
     }
 
     private MethodType explicitParams(final MethodType callSiteType) {

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/StoredScript.java

@@ -77,7 +77,7 @@ public final class StoredScript implements Serializable {
         return compilationId;
     }
 
-    private Map<String, Class<?>> installClasses(final Source source, final CodeInstaller<ScriptEnvironment> installer) {
+    private Map<String, Class<?>> installClasses(final Source source, final CodeInstaller installer) {
         final Map<String, Class<?>> installedClasses = new HashMap<>();
         final byte[]   mainClassBytes = classBytes.get(mainClassName);
         final Class<?> mainClass      = installer.install(mainClassName, mainClassBytes);
@@ -96,7 +96,7 @@ public final class StoredScript implements Serializable {
         return installedClasses;
     }
 
-    FunctionInitializer installFunction(final RecompilableScriptFunctionData data, final CodeInstaller<ScriptEnvironment> installer) {
+    FunctionInitializer installFunction(final RecompilableScriptFunctionData data, final CodeInstaller installer) {
         final Map<String, Class<?>> installedClasses = installClasses(data.getSource(), installer);
 
         assert initializers != null;
@@ -124,7 +124,7 @@ public final class StoredScript implements Serializable {
      * @param installer the installer
      * @return main script class
      */
-    Class<?> installScript(final Source source, final CodeInstaller<ScriptEnvironment> installer) {
+    Class<?> installScript(final Source source, final CodeInstaller installer) {
 
         final Map<String, Class<?>> installedClasses = installClasses(source, installer);
 

nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/tools/Shell.java

@@ -42,8 +42,8 @@ import java.util.ResourceBundle;
 import jdk.nashorn.api.scripting.NashornException;
 import jdk.nashorn.internal.codegen.Compiler;
 import jdk.nashorn.internal.codegen.Compiler.CompilationPhases;
-import jdk.nashorn.internal.ir.FunctionNode;
 import jdk.nashorn.internal.ir.Expression;
+import jdk.nashorn.internal.ir.FunctionNode;
 import jdk.nashorn.internal.ir.debug.ASTWriter;
 import jdk.nashorn.internal.ir.debug.PrintVisitor;
 import jdk.nashorn.internal.objects.Global;
@@ -255,12 +255,9 @@ public class Shell implements PartialParser {
                     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);
 

nashorn/test/script/trusted/JDK-8006529.js

@@ -120,7 +120,7 @@ var getEnvMethod = Context.class.getMethod("getEnv")
 
 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 @@ function compile(source, phases) {
     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);
 };