8290075: [JVMCI] only blessed methods can link against EventWriterFactory.getEventWriter

Reviewed-by: mgronlun

src/hotspot/share/jfr/instrumentation/jfrResolution.cpp

@@ -78,20 +78,32 @@ void JfrResolution::on_runtime_resolution(const CallInfo & info, TRAPS) {
   if (IS_METHOD_BLESSED(sender)) {
     return;
   }
+#if INCLUDE_JVMCI
+  // JVMCI compiler is doing linktime resolution
+  if (sender->method_holder()->name() == vmSymbols::jdk_vm_ci_hotspot_CompilerToVM()) {
+    if (sender->name()->equals("lookupMethodInPool")) {
+      return;
+    }
+  }
+#endif
   THROW_MSG(vmSymbols::java_lang_IllegalAccessError(), link_error_msg);
 }
 
+static inline bool is_compiler_linking_event_writer(const Symbol* holder, const Symbol* name) {
+  static const Symbol* const event_writer_factory_klass_name = vmSymbols::jdk_jfr_internal_event_EventWriterFactory();
+  assert(event_writer_factory_klass_name != nullptr, "invariant");
+  if (holder != event_writer_factory_klass_name) {
+    return false;
+  }
+  static const Symbol* const event_writer_method_name = vmSymbols::getEventWriter_name();
+  assert(event_writer_method_name != nullptr, "invariant");
+  return name == event_writer_method_name;
+}
+
 static inline bool is_compiler_linking_event_writer(const ciKlass * holder, const ciMethod * target) {
   assert(holder != nullptr, "invariant");
   assert(target != nullptr, "invariant");
-  static const Symbol* const event_writer_factory_klass_name = vmSymbols::jdk_jfr_internal_event_EventWriterFactory();
+  return is_compiler_linking_event_writer(holder->name()->get_symbol(), target->name()->get_symbol());
-  assert(event_writer_factory_klass_name != nullptr, "invariant");
-  if (holder->name()->get_symbol() != event_writer_factory_klass_name) {
-    return false;
-  }
-  static const Symbol* const event_writer_method_name = vmSymbols::getEventWriter_name();
-  assert(event_writer_method_name != nullptr, "invariant");
-  return target->name()->get_symbol() == event_writer_method_name;
 }
 
 #ifdef COMPILER1
@@ -111,3 +123,12 @@ void JfrResolution::on_c2_resolution(const Parse * parse, const ciKlass * holder
   }
 }
 #endif
+
+#if INCLUDE_JVMCI
+// JVMCI
+void JfrResolution::on_jvmci_resolution(const Method* caller, const Method* target, TRAPS) {
+  if (is_compiler_linking_event_writer(target->method_holder()->name(), target->name()) && !IS_METHOD_BLESSED(caller)) {
+    THROW_MSG(vmSymbols::java_lang_IllegalAccessError(), link_error_msg);
+  }
+}
+#endif

src/hotspot/share/jfr/instrumentation/jfrResolution.hpp

@@ -39,6 +39,7 @@ class JfrResolution : AllStatic {
   static void on_runtime_resolution(const CallInfo & info, TRAPS);
   static void on_c1_resolution(const GraphBuilder * builder, const ciKlass * holder, const ciMethod * target);
   static void on_c2_resolution(const Parse * parse, const ciKlass * holder, const ciMethod * target);
+  static void on_jvmci_resolution(const Method* caller, const Method* target, TRAPS);
 };
 
 #endif // SHARE_JFR_INSTRUMENTATION_JFRRESOLUTION_HPP

src/hotspot/share/jfr/jfr.cpp

@@ -116,6 +116,12 @@ void Jfr::on_resolution(const Parse* parse, const ciKlass* holder, const ciMetho
 }
 #endif
 
+#if INCLUDE_JVMCI
+void Jfr::on_resolution(const Method* caller, const Method* target, TRAPS) {
+  JfrResolution::on_jvmci_resolution(caller, target, CHECK);
+}
+#endif
+
 void Jfr::on_vm_shutdown(bool exception_handler, bool halt) {
   if (!halt && JfrRecorder::is_recording()) {
     JfrEmergencyDump::on_vm_shutdown(exception_handler);

src/hotspot/share/jfr/jfr.hpp

@@ -63,6 +63,7 @@ class Jfr : AllStatic {
   static void on_resolution(const CallInfo& info, TRAPS);
   static void on_resolution(const Parse* parse, const ciKlass* holder, const ciMethod* target);
   static void on_resolution(const GraphBuilder* builder, const ciKlass* holder, const ciMethod* target);
+  static void on_resolution(const Method* caller, const Method* target, TRAPS);
   static void on_java_thread_start(JavaThread* starter, JavaThread* startee);
   static void on_set_current_thread(JavaThread* jt, oop thread);
   static void on_vm_shutdown(bool exception_handler = false, bool halt = false);

src/hotspot/share/jvmci/jvmciCompilerToVM.cpp

@@ -63,6 +63,9 @@
 #include "runtime/timerTrace.hpp"
 #include "runtime/vframe_hp.hpp"
 #include "runtime/vframe.inline.hpp"
+#if INCLUDE_JFR
+#include "jfr/jfr.hpp"
+#endif
 
 JVMCIKlassHandle::JVMCIKlassHandle(Thread* thread, Klass* klass) {
   _thread = thread;
@@ -754,11 +757,13 @@ C2V_VMENTRY_NULL(jobject, lookupAppendixInPool, (JNIEnv* env, jobject, ARGUMENT_
   return JVMCIENV->get_jobject(JVMCIENV->get_object_constant(appendix_oop));
 C2V_END
 
-C2V_VMENTRY_NULL(jobject, lookupMethodInPool, (JNIEnv* env, jobject, ARGUMENT_PAIR(cp), jint index, jbyte opcode))
+C2V_VMENTRY_NULL(jobject, lookupMethodInPool, (JNIEnv* env, jobject, ARGUMENT_PAIR(cp), jint index, jbyte opcode, ARGUMENT_PAIR(caller)))
   constantPoolHandle cp(THREAD, UNPACK_PAIR(ConstantPool, cp));
+  methodHandle caller(THREAD, UNPACK_PAIR(Method, caller));
   InstanceKlass* pool_holder = cp->pool_holder();
   Bytecodes::Code bc = (Bytecodes::Code) (((int) opcode) & 0xFF);
   methodHandle method(THREAD, JVMCIRuntime::get_method_by_index(cp, index, bc, pool_holder));
+  JFR_ONLY(if (method.not_null()) Jfr::on_resolution(caller(), method(), CHECK_NULL);)
   JVMCIObject result = JVMCIENV->get_jvmci_method(method, JVMCI_CHECK_NULL);
   return JVMCIENV->get_jobject(result);
 C2V_END
@@ -2834,7 +2839,7 @@ JNINativeMethod CompilerToVM::methods[] = {
   {CC "lookupKlassRefIndexInPool",                    CC "(" HS_CONSTANT_POOL2 "I)I",                                                       FN_PTR(lookupKlassRefIndexInPool)},
   {CC "lookupKlassInPool",                            CC "(" HS_CONSTANT_POOL2 "I)Ljava/lang/Object;",                                      FN_PTR(lookupKlassInPool)},
   {CC "lookupAppendixInPool",                         CC "(" HS_CONSTANT_POOL2 "I)" OBJECTCONSTANT,                                         FN_PTR(lookupAppendixInPool)},
-  {CC "lookupMethodInPool",                           CC "(" HS_CONSTANT_POOL2 "IB)" HS_METHOD,                                             FN_PTR(lookupMethodInPool)},
+  {CC "lookupMethodInPool",                           CC "(" HS_CONSTANT_POOL2 "IB" HS_METHOD2 ")" HS_METHOD,                               FN_PTR(lookupMethodInPool)},
   {CC "constantPoolRemapInstructionOperandFromCache", CC "(" HS_CONSTANT_POOL2 "I)I",                                                       FN_PTR(constantPoolRemapInstructionOperandFromCache)},
   {CC "resolveBootstrapMethod",                       CC "(" HS_CONSTANT_POOL2 "I)[" OBJECT,                                                FN_PTR(resolveBootstrapMethod)},
   {CC "resolvePossiblyCachedConstantInPool",          CC "(" HS_CONSTANT_POOL2 "I)" JAVACONSTANT,                                           FN_PTR(resolvePossiblyCachedConstantInPool)},

src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.code/src/jdk/vm/ci/code/StackLockValue.java

@@ -24,7 +24,6 @@ package jdk.vm.ci.code;
 
 import jdk.vm.ci.meta.AllocatableValue;
 import jdk.vm.ci.meta.JavaValue;
-import jdk.vm.ci.meta.Value;
 
 /**
  * Represents lock information in the debug information.

src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/CompilerToVM.java

@@ -344,13 +344,21 @@ final class CompilerToVM {
      *            {@code -1}. If non-negative, then resolution checks specific to the bytecode it
      *            denotes are performed if the method is already resolved. Should any of these
      *            checks fail, 0 is returned.
+     * @param caller if non-null, do access checks in the context of {@code caller} calling the
+     *            looked up method
      * @return the resolved method entry, 0 otherwise
      */
-    HotSpotResolvedJavaMethodImpl lookupMethodInPool(HotSpotConstantPool constantPool, int cpi, byte opcode) {
+    HotSpotResolvedJavaMethodImpl lookupMethodInPool(HotSpotConstantPool constantPool, int cpi, byte opcode, HotSpotResolvedJavaMethodImpl caller) {
-        return lookupMethodInPool(constantPool, constantPool.getConstantPoolPointer(), cpi, opcode);
+        long callerMethodPointer = caller == null ? 0L : caller.getMethodPointer();
+        return lookupMethodInPool(constantPool, constantPool.getConstantPoolPointer(), cpi, opcode, caller, callerMethodPointer);
     }
 
-    private native HotSpotResolvedJavaMethodImpl lookupMethodInPool(HotSpotConstantPool constantPool, long constantPoolPointer, int cpi, byte opcode);
+    private native HotSpotResolvedJavaMethodImpl lookupMethodInPool(HotSpotConstantPool constantPool,
+                    long constantPoolPointer,
+                    int cpi,
+                    byte opcode,
+                    HotSpotResolvedJavaMethodImpl caller,
+                    long callerMethodPointer);
 
     /**
      * Ensures that the type referenced by the specified {@code JVM_CONSTANT_InvokeDynamic} entry at
@@ -513,7 +521,7 @@ final class CompilerToVM {
         try (HotSpotCompiledCodeStream stream = new HotSpotCompiledCodeStream(compiledCode, withTypeInfo, withComments, withMethods)) {
             return installCode0(stream.headChunk, stream.timeNS, withTypeInfo, compiledCode, stream.objectPool, code, failedSpeculationsAddress, speculations);
         }
-     }
+    }
 
     native int installCode0(long compiledCodeBuffer,
                     long serializationNS,

src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotConstantPool.java

@@ -689,9 +689,9 @@ public final class HotSpotConstantPool implements ConstantPool, MetaspaceHandleO
     }
 
     @Override
-    public JavaMethod lookupMethod(int cpi, int opcode) {
+    public JavaMethod lookupMethod(int cpi, int opcode, ResolvedJavaMethod caller) {
         final int index = rawIndexToConstantPoolCacheIndex(cpi, opcode);
-        final HotSpotResolvedJavaMethod method = compilerToVM().lookupMethodInPool(this, index, (byte) opcode);
+        final HotSpotResolvedJavaMethod method = compilerToVM().lookupMethodInPool(this, index, (byte) opcode, (HotSpotResolvedJavaMethodImpl) caller);
         if (method != null) {
             return method;
         } else {

src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.hotspot/src/jdk/vm/ci/hotspot/HotSpotJVMCICompilerConfig.java

@@ -22,11 +22,12 @@
  */
 package jdk.vm.ci.hotspot;
 
+import static jdk.vm.ci.services.Services.IS_IN_NATIVE_IMAGE;
+
 import java.util.List;
 import java.util.Set;
 
 import jdk.vm.ci.code.CompilationRequest;
-import jdk.vm.ci.common.JVMCIError;
 import jdk.vm.ci.common.NativeImageReinitialize;
 import jdk.vm.ci.hotspot.HotSpotJVMCIRuntime.Option;
 import jdk.vm.ci.runtime.JVMCICompiler;
@@ -36,8 +37,6 @@ import jdk.vm.ci.services.JVMCIPermission;
 import jdk.vm.ci.services.JVMCIServiceLocator;
 import jdk.vm.ci.services.Services;
 
-import static jdk.vm.ci.services.Services.IS_IN_NATIVE_IMAGE;
-
 final class HotSpotJVMCICompilerConfig {
 
     /**

src/jdk.internal.vm.ci/share/classes/jdk.vm.ci.meta/src/jdk/vm/ci/meta/ConstantPool.java

@@ -85,7 +85,26 @@ public interface ConstantPool {
      * @return a reference to the method at {@code cpi} in this pool
      * @throws ClassFormatError if the entry at {@code cpi} is not a method
      */
-    JavaMethod lookupMethod(int cpi, int opcode);
+    default JavaMethod lookupMethod(int cpi, int opcode) {
+        return lookupMethod(cpi, opcode, null);
+    }
+
+    /**
+     * Looks up a reference to a method. If {@code opcode} is non-negative, then resolution checks
+     * specific to the bytecode it denotes are performed if the method is already resolved. Should
+     * any of these checks fail, an unresolved method reference is returned.
+     *
+     * @param cpi the constant pool index
+     * @param opcode the opcode of the instruction for which the lookup is being performed or
+     *            {@code -1}
+     * @param caller if non-null, do access checks in the context of {@code caller} calling the
+     *            looked up method
+     * @return a reference to the method at {@code cpi} in this pool
+     * @throws ClassFormatError if the entry at {@code cpi} is not a method
+     * @throws IllegalAccessError if {@code caller} is non-null and it cannot link against the
+     *             looked up method
+     */
+    JavaMethod lookupMethod(int cpi, int opcode, ResolvedJavaMethod caller);
 
     /**
      * The details for invoking a bootstrap method associated with a {@code CONSTANT_Dynamic_info}

test/hotspot/jtreg/compiler/jvmci/common/patches/jdk.internal.vm.ci/jdk/vm/ci/hotspot/CompilerToVMHelper.java

@@ -128,7 +128,8 @@ public class CompilerToVMHelper {
 
     public static HotSpotResolvedJavaMethod lookupMethodInPool(
             ConstantPool constantPool, int cpi, byte opcode) {
-        return CTVM.lookupMethodInPool((HotSpotConstantPool) constantPool, cpi, opcode);
+        HotSpotResolvedJavaMethodImpl caller = null;
+        return CTVM.lookupMethodInPool((HotSpotConstantPool) constantPool, cpi, opcode, null);
     }
 
     public static void resolveInvokeDynamicInPool(

test/jdk/jdk/jfr/jvm/TestGetEventWriter.java

@@ -32,12 +32,18 @@ import java.util.List;
 import jdk.jfr.Event;
 import jdk.jfr.FlightRecorder;
 import jdk.jfr.Recording;
+import jdk.vm.ci.meta.MetaAccessProvider;
+import jdk.vm.ci.meta.ResolvedJavaMethod;
+import jdk.vm.ci.meta.ConstantPool;
+import jdk.vm.ci.runtime.JVMCI;
 
 /**
  * @test TestGetEventWriter
  * @key jfr
  * @requires vm.hasJFR
  * @library /test/lib
+ * @modules jdk.internal.vm.ci/jdk.vm.ci.meta
+ *          jdk.internal.vm.ci/jdk.vm.ci.runtime
  *
  * @compile PlaceholderEventWriter.java
  * @compile PlaceholderEventWriterFactory.java
@@ -51,6 +57,9 @@ import jdk.jfr.Recording;
  *
  * @run main/othervm jdk.jfr.jvm.TestGetEventWriter
  *
+ * @run main/othervm -XX:+UnlockExperimentalVMOptions -XX:+EnableJVMCI -Dtest.jvmci=true --add-exports=jdk.jfr/jdk.jfr.internal.event=ALL-UNNAMED
+ *      jdk.jfr.jvm.TestGetEventWriter
+ *
  * @run main/othervm/timeout=300 -Xint -XX:+UseInterpreter -Dinterpreted=true
  *      jdk.jfr.jvm.TestGetEventWriter
  *
@@ -101,6 +110,7 @@ public class TestGetEventWriter {
             throw new RuntimeException("Should not reach here");
         } catch (IllegalAccessError iae) {
             // OK, as expected
+            maybeCheckJVMCI(e.getClass(), "commit");
             return;
         }
     }
@@ -115,6 +125,7 @@ public class TestGetEventWriter {
             throw new RuntimeException("Should not reach here");
         } catch (IllegalAccessError iae) {
             // OK, as expected
+            maybeCheckJVMCI(e.getClass(), "commit");
             return;
         }
     }
@@ -134,6 +145,7 @@ public class TestGetEventWriter {
             throw new RuntimeException("Should not reach here");
         } catch (IllegalAccessError iae) {
             // OK, as expected
+            maybeCheckJVMCI(e.getClass(), "commit");
         }
         try {
             FlightRecorder.register(e.getClass());
@@ -154,6 +166,7 @@ public class TestGetEventWriter {
             throw new RuntimeException("Should not reach here");
         } catch (IllegalAccessError iae) {
             // OK, as expected
+            maybeCheckJVMCI(e.getClass(), "myCommit");
             return;
         }
     }
@@ -170,6 +183,7 @@ public class TestGetEventWriter {
             throw new RuntimeException("Should not reach here");
         } catch (IllegalAccessError iae) {
             // OK, as expected
+            maybeCheckJVMCI(e.getClass(), "myCommit");
         }
         // Instrumentation added.
         FlightRecorder.register(e.getClass().asSubclass(Event.class));
@@ -187,6 +201,7 @@ public class TestGetEventWriter {
             throw new RuntimeException("Should not reach here");
         } catch (IllegalAccessError iae) {
             // OK, as expected
+            maybeCheckJVMCI(e.getClass(), "commit");
         }
     }
 
@@ -317,4 +332,50 @@ public class TestGetEventWriter {
         System.out.println("About to invoke " + fullName + ".commit()");
         return (T) constructor.newInstance();
     }
+
+    private static ResolvedJavaMethod findCommitMethod(MetaAccessProvider metaAccess, Class<?> eventClass, String commitName) {
+        for (Method m : eventClass.getMethods()) {
+            if (m.getName().equals(commitName)) {
+                return metaAccess.lookupJavaMethod(m);
+            }
+        }
+        throw new AssertionError("could not find " + commitName + " method in " + eventClass);
+    }
+
+    // Factor out test.jvmci system property check to reduce unecessary work in -Xcomp.
+    private static void maybeCheckJVMCI(Class<?> eventClass, String commitName) throws Throwable {
+        if (!Boolean.getBoolean("test.jvmci")) {
+            return;
+        }
+        checkJVMCI(eventClass, commitName);
+    }
+
+    /**
+     * Checks that JVMCI prevents unblessed access to {@code EventWriterFactory.getEventWriter(long)}.
+     */
+    private static void checkJVMCI(Class<?> eventClass, String commitName) throws Throwable {
+        MetaAccessProvider metaAccess = JVMCI.getRuntime().getHostJVMCIBackend().getMetaAccess();
+        ResolvedJavaMethod commit = findCommitMethod(metaAccess, eventClass, commitName);
+        ConstantPool cp = commit.getConstantPool();
+
+        // Search for first INVOKESTATIC instruction in commit method which is expected
+        // to be the call to jdk.jfr.internal.event.EventWriterFactory.getEventWriter(long).
+        final int INVOKESTATIC = 184;
+        byte[] code = commit.getCode();
+        for (int bci = 0; bci < code.length; bci++) {
+            int b = code[bci] & 0xff;
+            if (b == INVOKESTATIC) {
+                int cpi = ((code[bci + 1] & 0xff) << 8) | (code[bci + 2] & 0xff);
+                try {
+                    cp.lookupMethod(cpi, 184, commit);
+                    throw new AssertionError("Expected IllegalAccessError");
+                } catch (IllegalAccessError e) {
+                }
+
+                // Ignore all subsequent instructions
+                return;
+            }
+        }
+        throw new AssertionError(eventClass + ": did not find INVOKESTATIC in " + commit.format("%H.%n(%p)"));
+    }
 }