[GR-28148] Handle class redefinition for method handle invocations.

PullRequest: graal/8340
xjjdlut · Feb 24, 2021 · 56a9a70 · 56a9a70
2 parents 2ec84b2 + 6fafe9f
commit 56a9a70
Show file tree

Hide file tree

Showing 7 changed files with 65 additions and 131 deletions.
diff --git a/...c/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/impl/ClassRedefinition.java b/...c/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/impl/ClassRedefinition.java
@@ -22,9 +22,18 @@
  */
 package com.oracle.truffle.espresso.impl;
 
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.regex.Matcher;
+
 import com.oracle.truffle.api.Assumption;
 import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
+import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
 import com.oracle.truffle.api.Truffle;
 import com.oracle.truffle.espresso.bytecode.BytecodeStream;
 import com.oracle.truffle.espresso.bytecode.Bytecodes;
@@ -40,20 +49,13 @@
 import com.oracle.truffle.espresso.jdwp.api.ErrorCodes;
 import com.oracle.truffle.espresso.jdwp.api.Ids;
 import com.oracle.truffle.espresso.jdwp.api.KlassRef;
+import com.oracle.truffle.espresso.meta.Meta;
 import com.oracle.truffle.espresso.runtime.Attribute;
 import com.oracle.truffle.espresso.runtime.EspressoContext;
 import com.oracle.truffle.espresso.runtime.EspressoException;
 import com.oracle.truffle.espresso.runtime.StaticObject;
 import com.oracle.truffle.espresso.vm.InterpreterToVM;
 
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
-import java.util.regex.Matcher;
-
 public final class ClassRedefinition {
 
     @CompilationFinal static volatile RedefineAssumption current = new RedefineAssumption();
@@ -629,4 +631,26 @@ private static int doRedefineClass(ChangePacket packet, Ids<Object> ids, Espress
         oldKlass.redefineClass(packet, refreshSubClasses, ids);
         return 0;
     }
+
+    @TruffleBoundary
+    public static Method handleRemovedMethod(Method resolutionSeed, Klass accessingKlass) {
+        try {
+            lock();
+            Method replacementMethod = resolutionSeed.getDeclaringKlass().lookupMethod(resolutionSeed.getName(), resolutionSeed.getRawSignature(), accessingKlass);
+            Meta meta = resolutionSeed.getMeta();
+            if (replacementMethod == null) {
+                throw Meta.throwExceptionWithMessage(meta.java_lang_NoSuchMethodError,
+                                meta.toGuestString(resolutionSeed.getDeclaringKlass().getNameAsString() + "." + resolutionSeed.getName() + resolutionSeed.getRawSignature()) +
+                                                " was removed by class redefinition");
+            } else if (resolutionSeed.isStatic() != replacementMethod.isStatic()) {
+                String message = resolutionSeed.isStatic() ? "expected static method: " : "expected non-static method:" + replacementMethod.getName();
+                throw Meta.throwExceptionWithMessage(meta.java_lang_IncompatibleClassChangeError, message);
+            } else {
+                // Update to the latest version of the replacement method
+                return replacementMethod;
+            }
+        } finally {
+            unlock();
+        }
+    }
 }
diff --git a/...cle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/methodhandle/MHLinkToNode.java b/...cle.truffle.espresso/src/com/oracle/truffle/espresso/nodes/methodhandle/MHLinkToNode.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -35,6 +35,8 @@
 import com.oracle.truffle.espresso.descriptors.Signatures;
 import com.oracle.truffle.espresso.descriptors.Symbol;
 import com.oracle.truffle.espresso.descriptors.Symbol.Type;
+import com.oracle.truffle.espresso.impl.ClassRedefinition;
+import com.oracle.truffle.espresso.impl.Klass;
 import com.oracle.truffle.espresso.impl.Field;
 import com.oracle.truffle.espresso.impl.Method;
 import com.oracle.truffle.espresso.meta.EspressoError;
@@ -76,29 +78,36 @@ public abstract class MHLinkToNode extends MethodHandleIntrinsicNode {
     @Override
     public Object call(Object[] args) {
         assert (getMethod().isStatic());
-        Method target = linker.linkTo(getTarget(args), args);
-        Object[] basicArgs = unbasic(args, target.getParsedSignature(), 0, argCount - 1, hasReceiver);
-        Object result = executeCall(basicArgs, target);
+        Method resolutionSeed = getTarget(args);
+        Object[] basicArgs = unbasic(args, resolutionSeed.getParsedSignature(), 0, argCount - 1, hasReceiver);
+        // method might have been redefined or removed by redefinition
+        if (resolutionSeed.isRemovedByRedefition()) {
+            Klass receiverKlass = hasReceiver ? ((StaticObject) basicArgs[0]).getKlass() : resolutionSeed.getDeclaringKlass();
+            resolutionSeed = ClassRedefinition.handleRemovedMethod(resolutionSeed, receiverKlass);
+        }
+
+        Method target = linker.linkTo(resolutionSeed, args);
+        Object result = executeCall(basicArgs, target.getMethodVersion());
         return rebasic(result, target.getReturnKind());
     }
 
-    protected abstract Object executeCall(Object[] args, Method target);
+    protected abstract Object executeCall(Object[] args, Method.MethodVersion target);
 
-    public static boolean canInline(Method target, Method cachedTarget) {
-        return target.identity() == cachedTarget.identity();
+    public static boolean canInline(Method.MethodVersion target, Method.MethodVersion cachedTarget) {
+        return target.getMethod().identity() == cachedTarget.getMethod().identity();
     }
 
     @SuppressWarnings("unused")
     @Specialization(limit = "INLINE_CACHE_SIZE_LIMIT", guards = {"inliningEnabled()", "canInline(target, cachedTarget)"})
-    Object executeCallDirect(Object[] args, Method target,
-                    @Cached("target") Method cachedTarget,
+    Object executeCallDirect(Object[] args, Method.MethodVersion target,
+                    @Cached("target") Method.MethodVersion cachedTarget,
                     @Cached("create(target.getCallTarget())") DirectCallNode directCallNode) {
         hits.inc();
         return directCallNode.call(args);
     }
 
     @Specialization(replaces = "executeCallDirect")
-    Object executeCallIndirect(Object[] args, Method target,
+    Object executeCallIndirect(Object[] args, Method.MethodVersion target,
                     @Cached("create()") IndirectCallNode callNode) {
         miss.inc();
         return callNode.call(target.getCallTarget(), args);

diff --git a/...ffle.espresso/src/com/oracle/truffle/espresso/nodes/quick/invoke/InvokeInterfaceNode.java b/...ffle.espresso/src/com/oracle/truffle/espresso/nodes/quick/invoke/InvokeInterfaceNode.java
@@ -23,7 +23,6 @@
 package com.oracle.truffle.espresso.nodes.quick.invoke;
 
 import com.oracle.truffle.api.CompilerDirectives;
-import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
 import com.oracle.truffle.api.dsl.Cached;
 import com.oracle.truffle.api.dsl.Specialization;
 import com.oracle.truffle.api.frame.VirtualFrame;
@@ -85,7 +84,7 @@ protected static MethodVersion methodLookup(StaticObject receiver, Method resolu
         if (resolutionSeed.isRemovedByRedefition()) {
             // accept a slow path once the method has been removed
             // put method behind a boundary to avoid a deopt loop
-            return handleRemovedMethod(receiver, resolutionSeed);
+            return ClassRedefinition.handleRemovedMethod(resolutionSeed, receiver.getKlass()).getMethodVersion();
         }
 
         int iTableIndex = resolutionSeed.getITableIndex();
@@ -98,31 +97,6 @@ protected static MethodVersion methodLookup(StaticObject receiver, Method resolu
         return method.getMethodVersion();
     }
 
-    @TruffleBoundary
-    private static MethodVersion handleRemovedMethod(StaticObject receiver, Method resolutionSeed) {
-        // do not run while a redefinition is in progress
-        try {
-            ClassRedefinition.lock();
-            // first check to see if there's a compatible new method before
-            // bailing out with a NoSuchMethodError
-            Klass receiverKlass = receiver.getKlass();
-            Method method = receiverKlass.lookupMethod(resolutionSeed.getName(), resolutionSeed.getRawSignature(), receiverKlass);
-            Meta meta = resolutionSeed.getMeta();
-            if (method == null) {
-                throw Meta.throwExceptionWithMessage(meta.java_lang_NoSuchMethodError,
-                                meta.toGuestString(resolutionSeed.getDeclaringKlass().getNameAsString() + "." + resolutionSeed.getName() + resolutionSeed.getRawSignature()));
-            } else if (method.isStatic()) {
-                throw Meta.throwExceptionWithMessage(meta.java_lang_IncompatibleClassChangeError, "expected non-static method: " + method.getName());
-            } else if (!method.isPublic()) {
-                throw Meta.throwException(meta.java_lang_IllegalAccessError);
-            } else {
-                return method.getMethodVersion();
-            }
-        } finally {
-            ClassRedefinition.unlock();
-        }
-    }
-
     @Override
     public final int execute(VirtualFrame frame, long[] primitives, Object[] refs) {
         // Method signature does not change across methods.

diff --git a/...ruffle.espresso/src/com/oracle/truffle/espresso/nodes/quick/invoke/InvokeSpecialNode.java b/...ruffle.espresso/src/com/oracle/truffle/espresso/nodes/quick/invoke/InvokeSpecialNode.java
@@ -24,15 +24,12 @@
 
 import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
-import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
 import com.oracle.truffle.api.frame.VirtualFrame;
 import com.oracle.truffle.api.nodes.DirectCallNode;
 import com.oracle.truffle.espresso.descriptors.Signatures;
 import com.oracle.truffle.espresso.impl.ClassRedefinition;
-import com.oracle.truffle.espresso.impl.Klass;
 import com.oracle.truffle.espresso.impl.Method;
 import com.oracle.truffle.espresso.impl.Method.MethodVersion;
-import com.oracle.truffle.espresso.meta.Meta;
 import com.oracle.truffle.espresso.nodes.BytecodeNode;
 import com.oracle.truffle.espresso.nodes.quick.QuickNode;
 import com.oracle.truffle.espresso.runtime.StaticObject;
@@ -52,13 +49,16 @@ public InvokeSpecialNode(Method method, int top, int callerBCI) {
 
     @Override
     public int execute(VirtualFrame frame, long[] primitives, Object[] refs) {
+        Object[] args = BytecodeNode.popArguments(primitives, refs, top, true, method.getMethod().getParsedSignature());
+        nullCheck((StaticObject) args[0]); // nullcheck receiver
         if (!method.getAssumption().isValid()) {
             // update to the latest method version and grab a new direct call target
             CompilerDirectives.transferToInterpreterAndInvalidate();
             if (removedByRedefintion()) {
                 // accept a slow path once the method has been removed
                 // put method behind a boundary to avoid a deopt loop
-                handleRemovedMethod();
+                Method resolutionSeed = method.getMethod();
+                method = ClassRedefinition.handleRemovedMethod(resolutionSeed, ((StaticObject) args[0]).getKlass()).getMethodVersion();
             } else {
                 method = method.getMethod().getMethodVersion();
             }
@@ -67,35 +67,10 @@ public int execute(VirtualFrame frame, long[] primitives, Object[] refs) {
             adoptChildren();
         }
         // TODO(peterssen): IsNull Node?
-        Object[] args = BytecodeNode.popArguments(primitives, refs, top, true, method.getMethod().getParsedSignature());
-        nullCheck((StaticObject) args[0]); // nullcheck receiver
         Object result = directCallNode.call(args);
         return (getResultAt() - top) + BytecodeNode.putKind(primitives, refs, getResultAt(), result, method.getMethod().getReturnKind());
     }
 
-    @TruffleBoundary
-    private void handleRemovedMethod() {
-        try {
-            ClassRedefinition.lock();
-
-            Method resolutionSeed = method.getMethod();
-            Klass accessingKlass = resolutionSeed.getDeclaringKlass();
-            Method replacementMethod = resolutionSeed.getDeclaringKlass().lookupMethod(resolutionSeed.getName(), resolutionSeed.getRawSignature(), accessingKlass);
-            Meta meta = resolutionSeed.getMeta();
-            if (replacementMethod == null) {
-                throw Meta.throwExceptionWithMessage(meta.java_lang_NoSuchMethodError,
-                                meta.toGuestString(resolutionSeed.getDeclaringKlass().getNameAsString() + "." + resolutionSeed.getName() + resolutionSeed.getRawSignature()));
-            } else if (replacementMethod.isStatic()) {
-                throw Meta.throwExceptionWithMessage(meta.java_lang_IncompatibleClassChangeError, "expected non-static method: " + replacementMethod.getName());
-            } else {
-                // Update to the latest version of the replacement method
-                method = replacementMethod.getMethodVersion();
-            }
-        } finally {
-            ClassRedefinition.unlock();
-        }
-    }
-
     @Override
     public boolean producedForeignObject(Object[] refs) {
         return method.getMethod().getReturnKind().isObject() && BytecodeNode.peekObject(refs, getResultAt()).isForeignObject();

diff --git a/...truffle.espresso/src/com/oracle/truffle/espresso/nodes/quick/invoke/InvokeStaticNode.java b/...truffle.espresso/src/com/oracle/truffle/espresso/nodes/quick/invoke/InvokeStaticNode.java
@@ -24,16 +24,13 @@
 
 import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
-import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
 import com.oracle.truffle.api.frame.VirtualFrame;
 import com.oracle.truffle.api.nodes.DirectCallNode;
 import com.oracle.truffle.espresso.descriptors.Signatures;
 import com.oracle.truffle.espresso.descriptors.Symbol.Name;
 import com.oracle.truffle.espresso.impl.ClassRedefinition;
-import com.oracle.truffle.espresso.impl.Klass;
 import com.oracle.truffle.espresso.impl.Method;
 import com.oracle.truffle.espresso.impl.Method.MethodVersion;
-import com.oracle.truffle.espresso.meta.Meta;
 import com.oracle.truffle.espresso.nodes.BytecodeNode;
 import com.oracle.truffle.espresso.nodes.EspressoRootNode;
 import com.oracle.truffle.espresso.nodes.quick.QuickNode;
@@ -66,7 +63,7 @@ public int execute(VirtualFrame frame, long[] primitives, Object[] refs) {
             if (removedByRedefintion()) {
                 // accept a slow path once the method has been removed
                 // put method behind a boundary to avoid a deopt loop
-                handleRemovedMethod(method);
+                method = ClassRedefinition.handleRemovedMethod(method.getMethod(), method.getMethod().getDeclaringKlass()).getMethodVersion();
             } else {
                 // update to the latest method version
                 method = method.getMethod().getMethodVersion();
@@ -94,30 +91,6 @@ public int execute(VirtualFrame frame, long[] primitives, Object[] refs) {
         return (getResultAt() - top) + BytecodeNode.putKind(primitives, refs, getResultAt(), result, method.getMethod().getReturnKind());
     }
 
-    @TruffleBoundary
-    private void handleRemovedMethod(MethodVersion methodVersion) {
-        try {
-            ClassRedefinition.lock();
-            Method resolutionSeed = methodVersion.getMethod();
-            // first check to see if there's a compatible new method before
-            // bailing out with an Error, e.g. due to changed modifiers
-            Klass accessingKlass = getBytecodesNode().getMethod().getDeclaringKlass();
-            Method replacementMethod = resolutionSeed.getDeclaringKlass().lookupMethod(resolutionSeed.getName(), resolutionSeed.getRawSignature(), accessingKlass);
-            Meta meta = resolutionSeed.getMeta();
-            if (replacementMethod == null) {
-                throw Meta.throwExceptionWithMessage(meta.java_lang_NoSuchMethodError,
-                                meta.toGuestString(resolutionSeed.getDeclaringKlass().getNameAsString() + "." + resolutionSeed.getName() + resolutionSeed.getRawSignature()));
-            } else if (!replacementMethod.isStatic()) {
-                throw Meta.throwExceptionWithMessage(meta.java_lang_IncompatibleClassChangeError, "expected static method: " + replacementMethod.getName());
-            } else {
-                // Update to the latest version of the replacement method
-                method = replacementMethod.getMethodVersion();
-            }
-        } finally {
-            ClassRedefinition.unlock();
-        }
-    }
-
     @Override
     public boolean producedForeignObject(Object[] refs) {
         return method.getMethod().getReturnKind().isObject() && BytecodeNode.peekObject(refs, getResultAt()).isForeignObject();

diff --git a/...ruffle.espresso/src/com/oracle/truffle/espresso/nodes/quick/invoke/InvokeVirtualNode.java b/...ruffle.espresso/src/com/oracle/truffle/espresso/nodes/quick/invoke/InvokeVirtualNode.java
@@ -23,7 +23,6 @@
 package com.oracle.truffle.espresso.nodes.quick.invoke;
 
 import com.oracle.truffle.api.CompilerDirectives;
-import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
 import com.oracle.truffle.api.dsl.Cached;
 import com.oracle.truffle.api.dsl.Specialization;
 import com.oracle.truffle.api.frame.VirtualFrame;
@@ -90,7 +89,7 @@ MethodVersion methodLookup(StaticObject receiver) {
         if (resolutionSeed.isRemovedByRedefition()) {
             // accept a slow path once the method has been removed
             // put method behind a boundary to avoid a deopt loop
-            return handleRemovedMethod(receiver, resolutionSeed);
+            return ClassRedefinition.handleRemovedMethod(resolutionSeed, receiver.getKlass()).getMethodVersion();
         }
 
         Klass receiverKlass = receiver.getKlass();
@@ -101,29 +100,6 @@ MethodVersion methodLookup(StaticObject receiver) {
         return receiverKlass.vtableLookup(vtableIndex).getMethodVersion();
     }
 
-    @TruffleBoundary
-    private static Method.MethodVersion handleRemovedMethod(StaticObject receiver, Method resolutionSeed) {
-        // do not run while a redefinition is in progress
-        try {
-            ClassRedefinition.lock();
-            // first check to see if there's a compatible new method before
-            // bailing out with a NoSuchMethodError
-            Klass receiverKlass = receiver.getKlass();
-            Method method = receiverKlass.lookupMethod(resolutionSeed.getName(), resolutionSeed.getRawSignature(), receiverKlass);
-            Meta meta = resolutionSeed.getMeta();
-            if (method == null) {
-                throw Meta.throwExceptionWithMessage(meta.java_lang_NoSuchMethodError,
-                                meta.toGuestString(resolutionSeed.getDeclaringKlass().getNameAsString() + "." + resolutionSeed.getName() + resolutionSeed.getRawSignature()));
-            } else if (method.isStatic()) {
-                throw Meta.throwExceptionWithMessage(meta.java_lang_IncompatibleClassChangeError, "expected non-static method: " + method.getName());
-            } else {
-                return method.getMethodVersion();
-            }
-        } finally {
-            ClassRedefinition.unlock();
-        }
-    }
-
     @Override
     public final int execute(VirtualFrame frame, long[] primitives, Object[] refs) {
         // Method signature does not change across methods.