diff --git a/robocode.host/src/main/java/net/sf/robocode/host/security/RobotClassLoader.java b/robocode.host/src/main/java/net/sf/robocode/host/security/RobotClassLoader.java index 624f704ac..feaf35a21 100644 --- a/robocode.host/src/main/java/net/sf/robocode/host/security/RobotClassLoader.java +++ b/robocode.host/src/main/java/net/sf/robocode/host/security/RobotClassLoader.java @@ -322,7 +322,8 @@ private void cleanStaticReferences(String className) { Class type = null; try { type = loadRobotClassLocally(className, false); - if (type != null) { + // Setting a static field initializes the class, which would run its static initializers on this thread + if (type != null && isInitialized(type)) { for (Field field : getAllFields(new ArrayList(), type)) { if (isStaticReference(field)) { cleanStaticReference(field); @@ -410,6 +411,26 @@ private void cleanStaticReference(Field field) { } } + /** + * Checks if a class has been initialized, without initializing it. + * + * @param type the class to check. + * @return true if the class has been initialized; false if not, or if this cannot be checked, as sun.misc.Unsafe + * might not have shouldBeInitialized() with newer Java versions. Static references cannot be cleaned with + * Java 12+ anyway. + */ + private static boolean isInitialized(Class type) { + try { + Field theUnsafe = Class.forName("sun.misc.Unsafe").getDeclaredField("theUnsafe"); + theUnsafe.setAccessible(true); + Object unsafe = theUnsafe.get(null); + + return !(Boolean) unsafe.getClass().getMethod("shouldBeInitialized", Class.class).invoke(unsafe, type); + } catch (Throwable t) { + return false; + } + } + /** * Gets all fields of a class (public, protected, private) and the ones inherited from all super classes. * diff --git a/robocode.host/src/main/java/net/sf/robocode/host/security/RobotClassRewriter.java b/robocode.host/src/main/java/net/sf/robocode/host/security/RobotClassRewriter.java index 15faada0f..37f1fb2a4 100644 --- a/robocode.host/src/main/java/net/sf/robocode/host/security/RobotClassRewriter.java +++ b/robocode.host/src/main/java/net/sf/robocode/host/security/RobotClassRewriter.java @@ -33,6 +33,9 @@ *
  • Creating a {@code FileInputStream}, {@code FileReader}, or {@code ObjectInputStream} is replaced with creating * the corresponding subclass in the {@code robocode.util} package, e.g. {@link robocode.util.RobotFileInputStream}, * which performs the checks.
    • + *
  • {@code finalize()} methods, and calls to them, except calls to the ones of Java classes, are renamed to + * {@code robocode$finalize()}, so they no longer override {@code Object.finalize()}. Otherwise, the JVM would call them + * on its finalizer thread, where robot code is not checked like on the robot's own threads.
    • * * The rewrite changes the existing method references in the constant pool, and changes the instructions using them, * i.e. {@code invokevirtual} into {@code invokestatic}, and {@code new} of the Java class into {@code new} of the @@ -137,6 +140,8 @@ private static void addSubclass(String className, String subclassName, String... SUBCLASS_CONSTRUCTORS.put(className, new HashSet(Arrays.asList(constructorDescriptors))); } + private static final String FINALIZE_NAME = "robocode$finalize"; + private static final int CONSTANT_Utf8 = 1; private static final int CONSTANT_Long = 5; private static final int CONSTANT_Double = 6; @@ -194,9 +199,13 @@ static ByteBuffer rewrite(ByteBuffer classFile) throws IOException { Map staticMethodClasses = new HashMap(); // methodref -> new class Map methodClasses = new HashMap(); // methodref -> new class, same name and type Map classSubclasses = new HashMap(); // class -> subclass + Set finalizeMethodRefs = new HashSet(); + boolean hasFinalize = false; for (int i = 1; i < count; i++) { - if (tags[i] == CONSTANT_Class) { + if ("finalize".equals(utf8s[i])) { + hasFinalize = true; // the class might declare a finalize() method + } else if (tags[i] == CONSTANT_Class) { String subclass = SUBCLASSES.get(utf8(utf8s, u2(bytes, offsets[i] + 1))); if (subclass != null) { classSubclasses.put(i, subclass); @@ -220,10 +229,14 @@ static ByteBuffer rewrite(ByteBuffer classFile) throws IOException { methodClasses.put(i, SUBCLASSES.get(className)); } else if (REPLACED_STATIC_METHODS.containsKey(className + '.' + name + descriptor)) { methodClasses.put(i, REPLACED_STATIC_METHODS.get(className + '.' + name + descriptor)); + } else if ("finalize".equals(name) && "()V".equals(descriptor) && !className.startsWith("java/")) { + // A call to a finalize() that the class only inherits, e.g. super.finalize() with robocode.Robot as + // superclass, throws NoSuchMethodError, but only if a robot calls it itself, as the JVM no longer does + finalizeMethodRefs.add(i); } } } - if (staticMethodNameIndexes.isEmpty() && methodClasses.isEmpty() && classSubclasses.isEmpty()) { + if (staticMethodNameIndexes.isEmpty() && methodClasses.isEmpty() && classSubclasses.isEmpty() && !hasFinalize) { return classFile; } @@ -245,6 +258,8 @@ static ByteBuffer rewrite(ByteBuffer classFile) throws IOException { for (Map.Entry entry : classSubclasses.entrySet()) { newClassIndexes.put(entry.getKey(), cp.addClass(entry.getValue())); } + int finalizeNameIndex = hasFinalize ? cp.addUtf8(FINALIZE_NAME) : 0; + int finalizeNameAndTypeIndex = hasFinalize ? cp.addNameAndType(finalizeNameIndex, "()V") : 0; byte[] newConstants = cp.toByteArray(); // Build the new class file @@ -266,6 +281,9 @@ static ByteBuffer rewrite(ByteBuffer classFile) throws IOException { putU2(result, offset + 3, nameAndTypeIndex); } } + for (int methodRef : finalizeMethodRefs) { + putU2(result, offsets[methodRef] + 3, finalizeNameAndTypeIndex); + } // Change the instructions Reader cr = new Reader(result); @@ -280,7 +298,13 @@ static ByteBuffer rewrite(ByteBuffer classFile) throws IOException { int methodsCount = cr.u2(); for (int m = 0; m < methodsCount; m++) { - cr.pos += 6; // access flags, name index, descriptor index + cr.u2(); // access flags + int methodNameOffset = cr.pos; + int methodNameIndex = cr.u2(); + int descriptorIndex = cr.u2(); + if ("finalize".equals(utf8s[methodNameIndex]) && "()V".equals(utf8s[descriptorIndex])) { + putU2(result, methodNameOffset, finalizeNameIndex); + } int attributesCount = cr.u2(); for (int a = 0; a < attributesCount; a++) { diff --git a/robocode.tests/src/test/java/net/sf/robocode/test/host/security/RobotClassLoaderTest.java b/robocode.tests/src/test/java/net/sf/robocode/test/host/security/RobotClassLoaderTest.java index 16654eb7b..7b138d07b 100644 --- a/robocode.tests/src/test/java/net/sf/robocode/test/host/security/RobotClassLoaderTest.java +++ b/robocode.tests/src/test/java/net/sf/robocode/test/host/security/RobotClassLoaderTest.java @@ -14,12 +14,18 @@ import net.sf.robocode.io.Logger; import net.sf.robocode.security.HiddenAccess; import net.sf.robocode.test.helpers.Assert; +import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import javax.tools.JavaCompiler; +import javax.tools.ToolProvider; import java.io.File; import java.io.IOException; +import java.lang.reflect.Method; import java.net.URL; +import java.nio.file.Files; +import java.nio.file.Path; /** @@ -89,6 +95,33 @@ public void robotBlockedRobocode() throws ClassNotFoundException { }); } + @Test + public void finalizeIsRenamed() throws Exception { + URL robotClassPath = compileRobot("finalizer", "Finalizer", "{\n" + + "\tprotected void finalize() throws Throwable { super.finalize(); }\n"); + + // The JVM must not call finalize() of robots on its finalizer thread + Class c = new RobotClassLoader(robotClassPath, "finalizer.Finalizer").loadClass("finalizer.Finalizer", true); + Method renamed = c.getDeclaredMethod("robocode$finalize"); + + renamed.setAccessible(true); + renamed.invoke(c.getDeclaredConstructor().newInstance()); // super.finalize() still works + Assert.assertThrows(NoSuchMethodException.class, () -> c.getDeclaredMethod("finalize")); + } + + // Compiles a class outside the class path of the tests, as its classes are otherwise treated as system classes + private static URL compileRobot(String packageName, String className, String body) throws IOException { + Path dir = Files.createTempDirectory("robots"); + Path source = Files.createDirectories(dir.resolve(packageName)).resolve(className + ".java"); + Files.write(source, ("package " + packageName + ";\n" + + "public class " + className + " " + body + "}\n").getBytes("UTF-8")); + JavaCompiler compiler = ToolProvider.getSystemJavaCompiler(); + Assumptions.assumeTrue(compiler != null); + Assert.assertEquals(0, compiler.run(null, null, null, "-nowarn", "-cp", System.getProperty("java.class.path"), + source.toString())); + return dir.toUri().toURL(); + } + @Test public void robotBlockedControl() throws ClassNotFoundException { Assert.assertThrows(ClassNotFoundException.class, () -> {