diff --git a/README.md b/README.md index f0ceb4c5..375de57d 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,7 @@ Advantages over Thermos: + Performance improvements + Updated libraries for newer plugin support * Implemented TimingsV2 -* Java 8–21 supported (using an integrated version of [lwjgl3ify](https://github.com/GTNewHorizons/lwjgl3ify)) +* Java 8–25 supported (using an integrated version of [lwjgl3ify](https://github.com/GTNewHorizons/lwjgl3ify)) + Backported Bukkit APIs (With some APIs requiring the companion mod [NecroTempus](https://github.com/CrucibleMC/NecroTempus)) + You can see more changes in the [releases](https://github.com/CrucibleMC/Crucible/releases) changelog. diff --git a/java9args.txt b/java9args.txt index 8f1a6db7..5b35f6c1 100644 --- a/java9args.txt +++ b/java9args.txt @@ -1,5 +1,4 @@ --illegal-access=warn --Djava.security.manager=allow -Dfile.encoding=UTF-8 -Dcrucible.weAreJava9=true --add-opens @@ -35,4 +34,4 @@ java.desktop/com.sun.imageio.plugins.png=ALL-UNNAMED jdk.dynalink/jdk.dynalink.beans=ALL-UNNAMED --add-modules java.sql.rowset --add-opens -java.sql.rowset/javax.sql.rowset.serial=ALL-UNNAMED \ No newline at end of file +java.sql.rowset/javax.sql.rowset.serial=ALL-UNNAMED diff --git a/src/main/java/io/github/crucible/CrucibleConfigs.java b/src/main/java/io/github/crucible/CrucibleConfigs.java index 9b651c0d..2ab5b400 100644 --- a/src/main/java/io/github/crucible/CrucibleConfigs.java +++ b/src/main/java/io/github/crucible/CrucibleConfigs.java @@ -186,6 +186,20 @@ public class CrucibleConfigs extends YamlConfig { @Comment("Prevents grass tick from loading Chunks!") public boolean crucible_noGrassChunkLoading = true; + @Comments({"Recompute stale StackMapTables as the last transform step so Mixin-heavy packs verify", + "on modern JDKs (Java 21+). COMPUTE_MAXS-only coremod/Mixin passes can leave frames the", + "split verifier rejects (VerifyError: Expecting a stackmap frame at branch target N).", + "Safe to leave on; only Java 8 (old type-inference verifier) does not need it.", + "The -Dcrucible.frameSafety=true|false system property overrides this if set."}) + public boolean crucible_asm_frameSafety = true; + + @Comments({"Only recompute classes whose deobf name starts with one of these comma-separated", + "prefixes. The stale frames come from passes that target net.minecraft.*, so that is the", + "default; recomputing everything would needlessly rewrite thousands of mod classes at load.", + "Use 'all' (or empty) to recompute every class, or add prefixes to widen to a mod package.", + "The -Dcrucible.frameSafety.scope=... system property overrides this if set."}) + public String crucible_asm_frameSafetyScope = "net.minecraft."; + @Comment("Let timings be turned on since the server statup!") public boolean timings_enabledSinceServerStartup = false; diff --git a/src/main/java/io/github/crucible/asm/AsmFrameSafetyTransformer.java b/src/main/java/io/github/crucible/asm/AsmFrameSafetyTransformer.java new file mode 100644 index 00000000..818e87d2 --- /dev/null +++ b/src/main/java/io/github/crucible/asm/AsmFrameSafetyTransformer.java @@ -0,0 +1,143 @@ +package io.github.crucible.asm; + +import java.io.File; +import java.io.FileOutputStream; +import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.List; + +import io.github.crucible.CrucibleConfigs; +import net.minecraft.launchwrapper.IClassTransformer; +import net.minecraft.launchwrapper.Launch; + +import org.objectweb.asm.ClassReader; +import org.objectweb.asm.ClassWriter; + +/** + * Recomputes StackMapTables ({@code COMPUTE_FRAMES}) as the last transform-chain step so classes + * left with stale frames by upstream {@code COMPUTE_MAXS} transformers pass the split bytecode + * verifier. Uses the deobf-aware, never-throwing {@link SafeAsmClassWriter}. Runs last by self-reordering + * to the tail of the LaunchClassLoader transformer list (whole-list copy-and-swap). Never breaks a class: + * any failure returns the input unchanged. + * + *

This complements RFB's {@code rfb-asm-safety} plugin, which only makes {@code getCommonSuperClass} + * safe when a transformer already recomputes frames; it does not force a recompute on a class + * (like Cauldron's patched {@code ChunkProviderServer.func_73153_a}) whose stale StackMapTable was left by + * a {@code COMPUTE_MAXS}-only coremod/Mixin pass. Reflectively loading that class (e.g. Dynmap's field + * scan) throws {@code VerifyError: Expecting a stackmap frame at branch target N}. Measured on the + * lwjgl3ify server path: the full pack fails this way on every modern JDK tested (21-25); Java 8 is + * unaffected because its verifier fails over to the old type-inference verifier for these class files. + * This transformer forces the repair. + * + *

Set {@code -Dcrucible.frameSafety.debug=true} to log every recompute outcome and dump the pre/post + * bytes of classes named in {@code -Dcrucible.frameSafety.debugClass=...} (default: ChunkProviderServer) + * to /tmp. Disable the whole transformer with {@code -Dcrucible.frameSafety=false}. + */ +public final class AsmFrameSafetyTransformer implements IClassTransformer { + + private static final Field TRANSFORMERS_FIELD = resolveTransformersField(); + private static final boolean DEBUG = Boolean.getBoolean("crucible.frameSafety.debug"); + private static final String DEBUG_CLASS = + System.getProperty("crucible.frameSafety.debugClass", "ChunkProviderServer"); + /** + * Only recompute classes whose (deobf) name starts with one of these comma-separated prefixes — + * the stale frames the verifier rejects come from COMPUTE_MAXS-only Mixin/coremod passes, + * which overwhelmingly target {@code net.minecraft.*}. Recomputing every class would needlessly read+ + * rewrite thousands of mod classes at load time. Configured via {@code crucible.asm.frameSafetyScope} + * in Crucible.yml ({@code all} or empty = everything). + */ + private static final String[] SCOPE = parseScope(CrucibleConfigs.configs.crucible_asm_frameSafetyScope); + + private static String[] parseScope(String raw) { + if (raw == null) return new String[0]; + raw = raw.trim(); + if (raw.isEmpty() || raw.equalsIgnoreCase("all")) return new String[0]; // empty = everything + String[] parts = raw.split(","); + for (int i = 0; i < parts.length; i++) parts[i] = parts[i].trim(); + return parts; + } + + private static boolean inScope(String transformedName) { + if (SCOPE.length == 0) return true; // "all" + if (transformedName == null) return false; + for (String p : SCOPE) if (!p.isEmpty() && transformedName.startsWith(p)) return true; + return false; + } + + private static Field resolveTransformersField() { + try { + Field f = Launch.classLoader.getClass().getDeclaredField("transformers"); + f.setAccessible(true); + return f; + } catch (Throwable t) { + return null; + } + } + + @Override + public byte[] transform(String name, String transformedName, byte[] basicClass) { + ensureRunsLast(); + if (basicClass == null) return null; + // never recompute our own writer/transformer, nor the external lwjgl3ify classes (which carry + // transformerExclusions and must reach the verifier untouched). + if (name != null && (name.startsWith("io.github.crucible.asm") + || name.startsWith("me.eigenraven.lwjgl3ify"))) return basicClass; + if (!inScope(transformedName)) return basicClass; + + final boolean dbg = DEBUG && transformedName != null && transformedName.contains(DEBUG_CLASS); + if (dbg) dump(transformedName + ".pre", basicClass); + try { + ClassReader reader = new ClassReader(basicClass); + SafeAsmClassWriter writer = new SafeAsmClassWriter(reader, ClassWriter.COMPUTE_FRAMES); + reader.accept(writer, 0); + byte[] out = writer.toByteArray(); + if (dbg) { + dump(transformedName + ".post", out); + System.out.println("[frameSafety] recomputed OK: " + transformedName); + } + return out; + } catch (Throwable t) { + if (dbg) { + System.out.println("[frameSafety] recompute THREW for " + transformedName + ": " + t); + t.printStackTrace(System.out); + } + return basicClass; + } + } + + private static void dump(String tag, byte[] bytes) { + try { + File f = new File("/tmp/framedbg-" + tag.replace('/', '.') + ".class"); + FileOutputStream fos = new FileOutputStream(f); + try { + fos.write(bytes); + } finally { + fos.close(); + } + System.out.println("[frameSafety] dumped " + f + " (" + bytes.length + " bytes)"); + } catch (Throwable ignored) { + // no-op + } + } + + @SuppressWarnings("unchecked") + private void ensureRunsLast() { + final Field f = TRANSFORMERS_FIELD; + if (f == null) return; + try { + final List current = (List) f.get(Launch.classLoader); + if (current == null || current.isEmpty()) return; + if (current.get(current.size() - 1) == this) return; + final int idx = current.indexOf(this); + if (idx < 0) return; + final List next = new ArrayList(current.size()); + for (int i = 0; i < current.size(); i++) { + if (i != idx) next.add(current.get(i)); + } + next.add(this); + f.set(Launch.classLoader, next); + } catch (Throwable ignored) { + // best-effort + } + } +} diff --git a/src/main/java/io/github/crucible/asm/SafeAsmClassWriter.java b/src/main/java/io/github/crucible/asm/SafeAsmClassWriter.java new file mode 100644 index 00000000..312a5ccf --- /dev/null +++ b/src/main/java/io/github/crucible/asm/SafeAsmClassWriter.java @@ -0,0 +1,240 @@ +package io.github.crucible.asm; + +import java.io.ByteArrayOutputStream; +import java.io.InputStream; +import java.lang.reflect.Method; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +import net.minecraft.launchwrapper.Launch; + +import org.objectweb.asm.ClassReader; +import org.objectweb.asm.ClassWriter; +import org.objectweb.asm.Opcodes; + +/** + * A {@link ClassWriter} whose {@link #getCommonSuperClass} resolves the type hierarchy without ever + * loading a class through {@link Class#forName} that could be duplicate-defined mid-transform, yet still + * resolves JDK types correctly. This is what lets {@code COMPUTE_FRAMES} repair the StackMapTables that + * upstream {@code COMPUTE_MAXS} transformers (FML / Mixin / pack coremods) leave invalid for the split + * bytecode verifier. + * + *

Resolution strategy per type, in order: + *

    + *
  1. {@code java.*} / {@code javax.*} / {@code sun.*} → {@code Class.forName} on the bootstrap/ + * system loader. These are already loaded, never transformed, and live on the bootstrap classpath + * where {@code Launch.classLoader.getResourceAsStream} can't see them — so a bytecode-only + * resolver silently fails on {@code java.lang.IllegalStateException} and merges exception types + * down to {@code Object}, which breaks MixinExtras' catch frames. forName is safe here precisely + * because these aren't the MC/mod classes that duplicate-define during transform.
  2. + *
  3. Everything else (MC + mods) → read the class header (super + interfaces) from the + * launch classloader's bytes (deobf-aware: SRG↔obf via FMLDeobfuscatingRemapper). Header-only, so it + * never links/initializes the class.
  4. + *
  5. Unresolvable → {@code java/lang/Object}. We never throw: throwing would abort the whole + * recompute and return the class with its broken frames intact.
  6. + *
+ */ +public final class SafeAsmClassWriter extends ClassWriter { + + private static final Map CACHE = new HashMap(); + + private static final Object REMAPPER; + private static final Method MAP; // obf -> srg + private static final Method UNMAP; // srg -> obf + + static { + Object instance = null; + Method map = null, unmap = null; + try { + Class c = Class.forName("cpw.mods.fml.common.asm.transformers.deobf.FMLDeobfuscatingRemapper"); + instance = c.getField("INSTANCE").get(null); + map = c.getMethod("map", String.class); + unmap = c.getMethod("unmap", String.class); + } catch (Throwable t) { + instance = null; + } + REMAPPER = instance; + MAP = map; + UNMAP = unmap; + } + + /** + * NOTE: deliberately does not pass the {@link ClassReader} to {@code super}. ASM's + * {@code ClassWriter(ClassReader, flags)} enables an optimization that copies unmodified methods + * byte-for-byte from the reader — including their original StackMapTable — which silently + * bypasses {@code COMPUTE_FRAMES} (the recompute becomes a no-op; input == output). Using + * {@code super(flags)} forces ASM to recompute frames for every method from scratch, which is the + * whole point of this writer. + */ + public SafeAsmClassWriter(ClassReader reader, int flags) { + super(flags); + } + + private static String remap(Method m, String name) { + if (REMAPPER == null || m == null || name == null) return name; + try { + Object r = m.invoke(REMAPPER, name); + return r != null ? (String) r : name; + } catch (Throwable t) { + return name; + } + } + + private static boolean isJdk(String internal) { + return internal.startsWith("java/") || internal.startsWith("javax/") || internal.startsWith("sun/") + || internal.startsWith("jdk/") || internal.startsWith("com/sun/"); + } + + @Override + protected String getCommonSuperClass(final String type1, final String type2) { + if (type1.equals(type2)) return type1; + if (type1.equals("java/lang/Object") || type2.equals("java/lang/Object")) return "java/lang/Object"; + + // Fast path: both JDK types -> use real reflection (safe; they're loaded & never transformed). + if (isJdk(type1) && isJdk(type2)) { + String r = jdkCommonSuper(type1, type2); + if (r != null) return r; + } + + final Node n1 = info(type1); + final Node n2 = info(type2); + if (n1 == null || n2 == null) return "java/lang/Object"; // never throw + + if (n1.isAssignableFrom(n2)) return type1; + if (n2.isAssignableFrom(n1)) return type2; + if (n1.isInterface || n2.isInterface) return "java/lang/Object"; + Node c = n1; + do { + c = c.superClass; + if (c == null) return "java/lang/Object"; + } while (!c.isAssignableFrom(n2)); + return c.name; + } + + /** Common superclass of two JDK types via the real classloader (they can't duplicate-define). */ + private static String jdkCommonSuper(String t1, String t2) { + try { + ClassLoader cl = ClassLoader.getSystemClassLoader(); + Class c1 = Class.forName(t1.replace('/', '.'), false, cl); + Class c2 = Class.forName(t2.replace('/', '.'), false, cl); + if (c1.isAssignableFrom(c2)) return t1; + if (c2.isAssignableFrom(c1)) return t2; + if (c1.isInterface() || c2.isInterface()) return "java/lang/Object"; + Class c = c1; + do { + c = c.getSuperclass(); + if (c == null) return "java/lang/Object"; + } while (!c.isAssignableFrom(c2)); + return c.getName().replace('.', '/'); + } catch (Throwable t) { + return null; + } + } + + private static Node info(final String type) { + synchronized (CACHE) { + if (CACHE.containsKey(type)) return CACHE.get(type); + } + Node node = build(type); + synchronized (CACHE) { + CACHE.put(type, node); + } + return node; + } + + private static Node build(final String type) { + // JDK types: resolve header via reflection (bootstrap classpath isn't visible as a resource). + if (isJdk(type)) { + try { + Class k = Class.forName(type.replace('/', '.'), false, ClassLoader.getSystemClassLoader()); + Node sup = k.getSuperclass() == null ? null : info(k.getSuperclass().getName().replace('.', '/')); + Set supers = new HashSet(); + if (sup != null) supers.addAll(sup.allSupers); + for (Class itf : k.getInterfaces()) { + Node in = info(itf.getName().replace('.', '/')); + if (in != null) supers.addAll(in.allSupers); + } + return new Node(type, sup, k.isInterface(), supers); + } catch (Throwable t) { + return null; + } + } + final byte[] bytes = bytesFor(type); + if (bytes == null) return null; + final ClassReader cr; + try { + cr = new ClassReader(bytes); + } catch (Throwable t) { + return null; + } + final Set supers = new HashSet(); + Node superNode = null; + final String superObf = cr.getSuperName(); + if (superObf != null) { + superNode = info(remap(MAP, superObf)); + if (superNode != null) supers.addAll(superNode.allSupers); + } + for (final String itfObf : cr.getInterfaces()) { + final Node in = info(remap(MAP, itfObf)); + if (in != null) supers.addAll(in.allSupers); + } + final boolean isInterface = (cr.getAccess() & Opcodes.ACC_INTERFACE) != 0; + return new Node(type, superNode, isInterface, supers); + } + + private static byte[] bytesFor(final String srgInternalName) { + try { + final byte[] b = Launch.classLoader.getClassBytes(srgInternalName.replace('/', '.')); + if (b != null) return b; + } catch (Throwable ignored) { + // fall through + } + final String obf = remap(UNMAP, srgInternalName); + InputStream is = null; + try { + is = Launch.classLoader.getResourceAsStream(obf + ".class"); + if (is == null && !obf.equals(srgInternalName)) { + is = Launch.classLoader.getResourceAsStream(srgInternalName + ".class"); + } + if (is != null) return readAll(is); + } catch (Throwable ignored) { + // fall through + } finally { + if (is != null) try { + is.close(); + } catch (Throwable ignored2) { + /* no-op */ + } + } + return null; + } + + private static byte[] readAll(final InputStream is) throws Exception { + final ByteArrayOutputStream bos = new ByteArrayOutputStream(8192); + final byte[] buf = new byte[8192]; + int n; + while ((n = is.read(buf)) != -1) bos.write(buf, 0, n); + return bos.toByteArray(); + } + + private static final class Node { + final String name; + final Node superClass; + final boolean isInterface; + final Set allSupers; + + Node(String name, Node superClass, boolean isInterface, Set allSupers) { + this.name = name; + this.superClass = superClass; + this.isInterface = isInterface; + this.allSupers = allSupers; + allSupers.add(this); + } + + boolean isAssignableFrom(Node other) { + return other.allSupers.contains(this); + } + } +} diff --git a/src/main/java/io/github/crucible/bootstrap/CrucibleCoremodHook.java b/src/main/java/io/github/crucible/bootstrap/CrucibleCoremodHook.java index 55827275..9989b6ee 100644 --- a/src/main/java/io/github/crucible/bootstrap/CrucibleCoremodHook.java +++ b/src/main/java/io/github/crucible/bootstrap/CrucibleCoremodHook.java @@ -1,6 +1,7 @@ package io.github.crucible.bootstrap; import cpw.mods.fml.common.launcher.FMLTweaker; +import io.github.crucible.CrucibleConfigs; import net.minecraft.launchwrapper.LaunchClassLoader; import java.io.File; @@ -15,9 +16,22 @@ public static void coremodHandleLaunch(File mcDir, LaunchClassLoader classLoader } catch (ClassNotFoundException e) { throw new RuntimeException(e); } - + classLoader.registerTransformer("io.github.crucible.patches.RecurrentComplexTransformer"); classLoader.registerTransformer("io.github.crucible.patches.StreamsTransformer"); classLoader.registerTransformer("thermos.ThermosClassTransformer"); + + // Force-recompute StackMapTables (kept LAST in the transformer chain) so classes left with stale + // frames by COMPUTE_MAXS-only coremod/Mixin passes pass the split bytecode verifier. RFB's + // rfb-asm-safety plugin only makes getCommonSuperClass safe for transformers that already + // recompute; it does NOT force a recompute on e.g. Cauldron's patched ChunkProviderServer + // .func_73153_a after a SpongePowered Mixin rewrites it with COMPUTE_MAXS only. Reflectively + // loading that class (Dynmap's field scan) then throws + // "VerifyError: Expecting a stackmap frame at branch target N". Measured on the lwjgl3ify server + // path: the full pack fails this way on every modern JDK tested (21-25); Java 8 is unaffected + // because its verifier fails over to the old type-inference verifier for these class files. + if (CrucibleConfigs.configs.crucible_asm_frameSafety) { + classLoader.registerTransformer("io.github.crucible.asm.AsmFrameSafetyTransformer"); + } } }