[GR-54852] Fix JDWP unexpected errors relating to array commands.

PullRequest: graal/18904

Author: javeleon

espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/api/JDWPContext.java

@@ -155,13 +155,14 @@ public interface JDWPContext {
     int getArrayLength(Object array);
 
     /**
-     * Returns the TypeTag constant for the input object. The TypeTag will be determined based on
-     * the declaring class of the object.
+     * Returns the tag constant for the component type of the input array. Note that all sub
+     * variants of the OBJECT tag constant is not used, e.g. if the component class of the array is
+     * j.l.String the generic OBJECT tag is returned and not the STRING tag.
      *
      * @param array must be a guest language array object
-     * @return TypeTag for the object
+     * @return the TypeTag for the component type of the array
      */
-    byte getTypeTag(Object array);
+    byte getArrayComponentTag(Object array);
 
     /**
      * Returns an unboxed host primitive type array of the array.
@@ -266,14 +267,6 @@ public interface JDWPContext {
      */
     boolean isValidClassLoader(Object object);
 
-    /**
-     * Converts an arbitrary host object to the corresponding guest object.
-     *
-     * @param object the host object to convert
-     * @return the guest object
-     */
-    Object toGuest(Object object);
-
     // temporarily needed until we get better exception-type based filtering in the Debug API
     Object getGuestException(Throwable exception);
 
@@ -510,4 +503,6 @@ public interface JDWPContext {
      * being reported via stepping).
      */
     boolean isSingleSteppingDisabled();
+
+    Object allocateInstance(KlassRef klass);
 }

espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/api/MethodRef.java

@@ -123,13 +123,30 @@ public interface MethodRef {
     LineNumberTableRef getLineNumberTable();
 
     /**
-     * Invokes the method on the input callee object with input arguments.
+     * Invokes a non-private, non-constructor instance method on the input callee object with input
+     * arguments. The first argument must be the self object.
      *
-     * @param callee guest-language object on which to execute the method
      * @param args guest-language arguments used when calling the method
      * @return the guest-language return value
      */
-    Object invokeMethod(Object callee, Object[] args);
+    Object invokeMethodVirtual(Object... args);
+
+    /**
+     * Invokes a static method with input arguments.
+     *
+     * @param args guest-language arguments used when calling the method
+     * @return the guest-language return value
+     */
+    Object invokeMethodStatic(Object... args);
+
+    /**
+     * Invokes a constructor or private instance method with input arguments. The first argument
+     * must be the self object.
+     *
+     * @param args guest-language arguments used when calling the method
+     * @return the guest-language return value
+     */
+    Object invokeMethodSpecial(Object... args);
 
     /**
      * Determines if the declaring class has a source file attribute.
@@ -211,4 +228,18 @@ public interface MethodRef {
      * @return last bci
      */
     long getLastBCI();
+
+    /**
+     * Determines if the method is a constructor.
+     *
+     * @return true if the method is a constructor
+     */
+    boolean isConstructor();
+
+    /**
+     * Determines if the method is a static initializer.
+     *
+     * @return true if the method is a static initializer
+     */
+    boolean isClassInitializer();
 }

espresso/src/com.oracle.truffle.espresso.jdwp/src/com/oracle/truffle/espresso/jdwp/impl/JDWP.java

@@ -1125,7 +1125,8 @@ static CommandResult createReply(Packet packet, DebuggerController controller, D
                 }
 
                 MethodRef method = verifyMethodRef(input.readLong(), reply, context);
-                if (method == null) {
+                if (method == null || !Modifier.isStatic(method.getModifiers()) || method.isClassInitializer()) {
+                    reply.errorCode(ErrorCodes.INVALID_METHODID);
                     return new CommandResult(reply);
                 }
 
@@ -1160,12 +1161,7 @@ static CommandResult createReply(Packet packet, DebuggerController controller, D
                 try {
                     // we have to call the method in the correct thread, so post a
                     // Callable to the controller and wait for the result to appear
-                    ThreadJob<Object> job = new ThreadJob<>(thread, new Callable<>() {
-                        @Override
-                        public Object call() {
-                            return method.invokeMethod(null, args);
-                        }
-                    }, suspensionStrategy);
+                    ThreadJob<Object> job = new ThreadJob<>(thread, () -> method.invokeMethodStatic(args), suspensionStrategy);
                     controller.postJobForThread(job);
 
                     // invocation of a method can cause events with possible thread suspension
@@ -1209,6 +1205,10 @@ static CommandResult createReply(Packet packet, DebuggerController controller) {
                 if (klass == null) {
                     return new CommandResult(reply);
                 }
+                if (klass.isArray() || klass.isInterface() || Modifier.isAbstract(klass.getModifiers())) {
+                    reply.errorCode(ErrorCodes.INVALID_CLASS);
+                    return new CommandResult(reply);
+                }
 
                 Object thread = verifyThread(input.readLong(), reply, context, true);
                 if (thread == null) {
@@ -1223,32 +1223,31 @@ static CommandResult createReply(Packet packet, DebuggerController controller) {
                 }
 
                 MethodRef method = verifyMethodRef(input.readLong(), reply, context);
-                if (method == null) {
-                    controller.warning(() -> "not a valid method");
+                if (method == null || !method.isConstructor() || method.getDeclaringKlassRef() != klass) {
+                    reply.errorCode(ErrorCodes.INVALID_METHODID);
                     return new CommandResult(reply);
                 }
 
                 controller.fine(() -> "trying to invoke constructor in klass: " + klass.getNameAsString());
 
                 int arguments = input.readInt();
 
-                Object[] args = new Object[arguments];
-                for (int i = 0; i < arguments; i++) {
+                Object[] args = new Object[arguments + 1];
+                // we leave room for the allocated object as the first arg
+                for (int i = 1; i < args.length; i++) {
                     byte valueKind = input.readByte();
                     args[i] = readValue(valueKind, input, context);
                 }
 
                 int invocationOptions = input.readInt();
                 byte suspensionStrategy = invocationOptions == 1 ? SuspendStrategy.EVENT_THREAD : SuspendStrategy.ALL;
                 try {
-                    // we have to call the method in the correct thread, so post a
+                    // we have to call the constructor in the correct thread, so post a
                     // Callable to the controller and wait for the result to appear
-                    ThreadJob<?> job = new ThreadJob<>(thread, new Callable<>() {
-
-                        @Override
-                        public Object call() throws Exception {
-                            return method.invokeMethod(null, args);
-                        }
+                    ThreadJob<?> job = new ThreadJob<>(thread, () -> {
+                        args[0] = context.allocateInstance(klass);
+                        method.invokeMethodSpecial(args);
+                        return args[0];
                     }, suspensionStrategy);
                     controller.postJobForThread(job);
                     ThreadJob<?>.JobResult<?> result = job.getResult();
@@ -1329,7 +1328,7 @@ static CommandResult createReply(Packet packet, DebuggerController controller, D
                     return new CommandResult(reply);
                 }
 
-                if (method.getDeclaringKlassRef() != itf) {
+                if (method.getDeclaringKlassRef() != itf || !Modifier.isStatic(method.getModifiers()) || method.isClassInitializer()) {
                     reply.errorCode(ErrorCodes.INVALID_METHODID);
                     return new CommandResult(reply);
                 }
@@ -1349,13 +1348,7 @@ static CommandResult createReply(Packet packet, DebuggerController controller, D
                 try {
                     // we have to call the method in the correct thread, so post a
                     // Callable to the controller and wait for the result to appear
-                    ThreadJob<Object> job = new ThreadJob<>(thread, new Callable<>() {
-
-                        @Override
-                        public Object call() throws Exception {
-                            return method.invokeMethod(null, args);
-                        }
-                    }, suspensionStrategy);
+                    ThreadJob<Object> job = new ThreadJob<>(thread, () -> method.invokeMethodStatic(args), suspensionStrategy);
                     controller.postJobForThread(job);
                     // invocation of a method can cause events with possible thread suspension
                     // to happen, e.g. class prepare events for newly loaded classes
@@ -1755,6 +1748,14 @@ static CommandResult createReply(Packet packet, DebuggerController controller, D
                 JDWPContext context = controller.getContext();
 
                 long objectId = input.readLong();
+
+                Object receiver = context.getIds().fromId((int) objectId);
+                if (receiver == null) {
+                    // object was garbage collected
+                    reply.errorCode(ErrorCodes.INVALID_OBJECT);
+                    return new CommandResult(reply);
+                }
+
                 long threadId = input.readLong();
 
                 Object thread = verifyThread(threadId, reply, context, true);
@@ -1775,42 +1776,38 @@ static CommandResult createReply(Packet packet, DebuggerController controller, D
                 }
 
                 long methodId = input.readLong();
-                int arguments = input.readInt();
-
-                Object[] args = new Object[arguments];
-                for (int i = 0; i < arguments; i++) {
-                    byte valueKind = input.readByte();
-                    args[i] = readValue(valueKind, input, context);
-                }
-
-                Object callee = context.getIds().fromId((int) objectId);
-                if (callee == null) {
-                    // object was garbage collected
-                    reply.errorCode(ErrorCodes.INVALID_OBJECT);
-                    return new CommandResult(reply);
-                }
                 MethodRef method = verifyMethodRef(methodId, reply, context);
 
                 if (method == null) {
                     return new CommandResult(reply);
                 }
 
-                if (!context.isMemberOf(callee, method.getDeclaringKlassRef())) {
+                if (Modifier.isStatic(method.getModifiers()) || method.isConstructor() || !context.isMemberOf(receiver, method.getDeclaringKlassRef())) {
                     reply.errorCode(ErrorCodes.INVALID_METHODID);
                     return new CommandResult(reply);
                 }
 
+                int arguments = input.readInt();
+
+                Object[] args = new Object[arguments + 1];
+                args[0] = receiver;
+                for (int i = 1; i < args.length; i++) {
+                    byte valueKind = input.readByte();
+                    args[i] = readValue(valueKind, input, context);
+                }
+
                 controller.fine(() -> "trying to invoke method: " + method.getNameAsString());
 
                 int invocationOptions = input.readInt();
                 byte suspensionStrategy = invocationOptions == 1 ? SuspendStrategy.EVENT_THREAD : SuspendStrategy.ALL;
                 try {
                     // we have to call the method in the correct thread, so post a
                     // Callable to the controller and wait for the result to appear
-                    ThreadJob<Object> job = new ThreadJob<>(thread, new Callable<>() {
-                        @Override
-                        public Object call() throws Exception {
-                            return method.invokeMethod(callee, args);
+                    ThreadJob<Object> job = new ThreadJob<>(thread, () -> {
+                        if (Modifier.isPrivate(method.getModifiers())) {
+                            return method.invokeMethodSpecial(args);
+                        } else {
+                            return method.invokeMethodVirtual(args);
                         }
                     }, suspensionStrategy);
                     controller.postJobForThread(job);
@@ -2539,7 +2536,7 @@ static CommandResult createReply(Packet packet, JDWPContext context) {
                     return new CommandResult(reply);
                 }
 
-                byte tag = context.getTypeTag(array);
+                byte tag = context.getArrayComponentTag(array);
                 boolean isPrimitive = TagConstants.isPrimitive(tag);
 
                 reply.writeByte(tag);
@@ -2570,64 +2567,28 @@ static CommandResult createReply(Packet packet, JDWPContext context) {
                     return new CommandResult(reply);
                 }
 
-                byte tag = context.getTypeTag(array);
+                byte tag = context.getArrayComponentTag(array);
 
                 setArrayValues(context, input, index, values, array, tag);
                 return new CommandResult(reply);
             }
 
             private static void setArrayValues(JDWPContext context, PacketStream input, int index, int values, Object array, byte tag) {
                 for (int i = index; i < index + values; i++) {
-                    switch (tag) {
-                        case TagConstants.BOOLEAN:
-                            boolean bool = input.readBoolean();
-                            byte[] boolArray = context.getUnboxedArray(array);
-                            boolArray[i] = bool ? (byte) 1 : (byte) 0;
-                            break;
-                        case TagConstants.BYTE:
-                            byte b = input.readByte();
-                            byte[] byteArray = context.getUnboxedArray(array);
-                            byteArray[i] = b;
-                            break;
-                        case TagConstants.SHORT:
-                            short s = input.readShort();
-                            short[] shortArray = context.getUnboxedArray(array);
-                            shortArray[i] = s;
-                            break;
-                        case TagConstants.CHAR:
-                            char c = input.readChar();
-                            char[] charArray = context.getUnboxedArray(array);
-                            charArray[i] = c;
-                            break;
-                        case TagConstants.INT:
-                            int j = input.readInt();
-                            int[] intArray = context.getUnboxedArray(array);
-                            intArray[i] = j;
-                            break;
-                        case TagConstants.FLOAT:
-                            float f = input.readFloat();
-                            float[] floatArray = context.getUnboxedArray(array);
-                            floatArray[i] = f;
-                            break;
-                        case TagConstants.LONG:
-                            long l = input.readLong();
-                            long[] longArray = context.getUnboxedArray(array);
-                            longArray[i] = l;
-                            break;
-                        case TagConstants.DOUBLE:
-                            double d = input.readDouble();
-                            double[] doubleArray = context.getUnboxedArray(array);
-                            doubleArray[i] = d;
-                            break;
-                        case TagConstants.ARRAY:
-                        case TagConstants.STRING:
-                        case TagConstants.OBJECT:
-                            Object value = context.getIds().fromId((int) input.readLong());
-                            context.setArrayValue(array, i, value);
-                            break;
-                        default:
-                            throw new RuntimeException("should not reach here");
-                    }
+                    Object value = switch (tag) {
+                        case TagConstants.BOOLEAN -> input.readBoolean();
+                        case TagConstants.BYTE -> input.readByte();
+                        case TagConstants.SHORT -> input.readShort();
+                        case TagConstants.CHAR -> input.readChar();
+                        case TagConstants.INT -> input.readInt();
+                        case TagConstants.FLOAT -> input.readFloat();
+                        case TagConstants.LONG -> input.readLong();
+                        case TagConstants.DOUBLE -> input.readDouble();
+                        case TagConstants.ARRAY, TagConstants.STRING, TagConstants.OBJECT ->
+                            context.getIds().fromId((int) input.readLong());
+                        default -> throw new RuntimeException("should not reach here: " + tag);
+                    };
+                    context.setArrayValue(array, i, value);
                 }
             }
         }
@@ -3084,7 +3045,7 @@ private static void writeMethodResult(PacketStream reply, JDWPContext context, T
                 controller.getGCPrevention().setActiveWhileSuspended(thread, guestException);
             }
         } else {
-            Object value = context.toGuest(result.getResult());
+            Object value = result.getResult();
             if (value != null) {
                 byte tag = context.getTag(value);
                 writeValue(tag, value, reply, true, context);