Mixins: Do not crash when a mixin has no associated handler

Initially it made sense to crash, but after some experience this is not
a viable solution. There is usually a bit of time between when the mixin
is created and when `.attach()` is called. If the mixin attempts to run
in between this period, it would lead to a crash, however this is not
user error.

To fix this, all generated mixins now check with `JSLoader` to see if
they have an attached method before attempting to invoke it. If there is
no attached method yet, the mixin will act as if it did not exist (e.g.
do nothing for inject mixins, call the original method for redirect
mixins, etc).

With this change, CT users can now inject into places that are invoked
early in the startup process, such as MinecraftClient.setScreen (which
is invoked in the MinecraftClient constructor).

src/main/java/com/chattriggers/ctjs/internal/mixins/MinecraftClientMixin.java

@@ -2,9 +2,14 @@
 
 import com.chattriggers.ctjs.internal.engine.CTEvents;
 import com.chattriggers.ctjs.api.triggers.TriggerType;
+import com.chattriggers.ctjs.internal.engine.JSLoader;
+import com.llamalad7.mixinextras.injector.wrapoperation.Operation;
+import com.llamalad7.mixinextras.injector.wrapoperation.WrapOperation;
 import net.minecraft.client.MinecraftClient;
+import net.minecraft.client.RunArgs;
 import net.minecraft.client.gui.screen.Screen;
 import net.minecraft.client.network.ServerInfo;
+import net.minecraft.client.resource.server.ServerResourcePackLoader;
 import net.minecraft.client.world.ClientWorld;
 import org.jetbrains.annotations.Nullable;
 import org.spongepowered.asm.mixin.Mixin;
@@ -13,6 +18,8 @@
 import org.spongepowered.asm.mixin.injection.Inject;
 import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;
 
+import java.nio.file.Path;
+
 @Mixin(MinecraftClient.class)
 public abstract class MinecraftClientMixin {
     @Shadow @Nullable public ClientWorld world;

src/main/kotlin/com/chattriggers/ctjs/internal/engine/JSLoader.kt

@@ -207,6 +207,9 @@ object JSLoader {
         }
     }
 
+    @JvmStatic
+    fun mixinIsAttached(id: Int) = mixinIdMap[id]?.method != null
+
     fun invokeMixinLookup(id: Int): MixinCallback {
         val callback = mixinIdMap[id] ?: error("Unknown mixin id $id for loader ${this::class.simpleName}")
 

src/main/kotlin/com/chattriggers/ctjs/internal/launch/DynamicMixinManager.kt

@@ -44,8 +44,8 @@ internal object DynamicMixinManager {
         for ((mixin, details) in mixins) {
             val ctx = GenerationContext(mixin)
             val generator = DynamicMixinGenerator(ctx, details)
-            ByteBasedStreamHandler[generator.generatedClassFullPath + ".class"] = generator.generate()
-            dynamicMixins += generator.generatedClassName
+            ByteBasedStreamHandler[ctx.generatedClassFullPath + ".class"] = generator.generate()
+            dynamicMixins += ctx.generatedClassName
         }
 
         ByteBasedStreamHandler[GENERATED_MIXIN] = createDynamicMixinsJson(dynamicMixins)

src/main/kotlin/com/chattriggers/ctjs/internal/launch/InvokeDynamicSupport.kt

@@ -68,20 +68,8 @@ internal object InvokeDynamicSupport {
         // Make an initial lookup to the target function. This is where we want our mixin handler method to point to
         val mixinCallback = JSLoader.invokeMixinLookup(mixinId)
 
-        check((mixinCallback.handle == null) == (mixinCallback.method == null))
-
-        val targetHandle = if (mixinCallback.handle == null) {
-            // If we don't have a handle, that means the user hasn't called attach() on the callback, meaning this mixin
-            // is "unused"...
-            val (methodName, injectionType) = InjectorGenerator.disassembleIndyName(name)
-
-            error(
-                "$injectionType mixin into method $methodName was called, but has no handler. Did you forget to " +
-                    "call attach()?"
-            )
-        } else {
-            mixinCallback.handle!!
-        }
+        checkNotNull(mixinCallback.handle)
+        checkNotNull(mixinCallback.method)
 
         // Until we /ct load, however. When we reload, we need to re-resolve all JS invocation targets since our old
         // engine context has been thrown away and recreated. It is also possible that the user has changed their code
@@ -96,14 +84,14 @@ internal object InvokeDynamicSupport {
         // Note that the mechanism for flipping these switches is in MixinCallback. When the user calls attach(), the
         // invalidator gets invalidated, as the method has changed. This of course happens for all mixins when the user
         // /ct loads, since the scripts are re-run.
-        val guardedTarget = mixinCallback.invalidator.guardWithTest(targetHandle, initTarget)
+        val guardedTarget = mixinCallback.invalidator.guardWithTest(mixinCallback.handle, initTarget)
 
         // We now have a target that is very fast to call back into the target method, and can survive reloads or calls
         // to attach(), so we want our call site to now point to that target.
         callSite.target = guardedTarget
 
         // This method invocation occurred because we actually tried to call the target method with the supplied mixin
         // arguments. So in addition to performing the call site rebinding, we also need to make the actual method call.
-        return targetHandle.invoke(args)
+        return mixinCallback.handle!!.invoke(args)
     }
 }

src/main/kotlin/com/chattriggers/ctjs/internal/launch/annotations.kt

@@ -1,6 +1,7 @@
 package com.chattriggers.ctjs.internal.launch
 
 import com.chattriggers.ctjs.internal.utils.descriptor
+import org.objectweb.asm.Opcodes
 import org.spongepowered.asm.mixin.injection.Constant as SPConstant
 
 /*
@@ -27,6 +28,86 @@ data class At(
     val opcode: Int?,
     val remap: Boolean?,
 ) {
+    internal val atTarget: AtTarget<*> by lazy(::getAtTarget)
+
+    internal sealed class AtTarget<T : Descriptor>(val descriptor: T, val targetName: String)
+
+    internal class InvokeTarget(descriptor: Descriptor.Method) : AtTarget<Descriptor.Method>(descriptor, "INVOKE") {
+        override fun toString() = descriptor.originalDescriptor()
+    }
+
+    internal class NewTarget(descriptor: Descriptor.New) : AtTarget<Descriptor.New>(descriptor, "NEW") {
+        override fun toString() = descriptor.originalDescriptor()
+    }
+
+    internal class FieldTarget(
+        descriptor: Descriptor.Field, val isGet: Boolean?, val isStatic: Boolean?,
+    ) : AtTarget<Descriptor.Field>(descriptor, "FIELD") {
+        override fun toString() = descriptor.originalDescriptor()
+    }
+
+    internal class ConstantTarget(val key: String, descriptor: Descriptor) : AtTarget<Descriptor>(descriptor, "CONSTANT") {
+        init {
+            require(descriptor.isType)
+        }
+
+        override fun toString() = "$key=$descriptor"
+    }
+
+    private fun getAtTarget(): AtTarget<*> {
+        return when (value) {
+            "INVOKE" -> {
+                requireNotNull(target) { "At targeting INVOKE expects its target to be a method descriptor" }
+                InvokeTarget(Descriptor.Parser(target).parseMethod(full = true))
+            }
+            "NEW" -> {
+                requireNotNull(target) { "At targeting NEW expects its target to be a new invocation descriptor" }
+                NewTarget(Descriptor.Parser(target).parseNew(full = true))
+            }
+            "FIELD" -> {
+                requireNotNull(target) { "At targeting FIELD expects its target to be a field descriptor" }
+                if (opcode != null) {
+                    require(
+                        opcode in setOf(
+                            Opcodes.GETFIELD,
+                            Opcodes.GETSTATIC,
+                            Opcodes.PUTFIELD,
+                            Opcodes.PUTSTATIC
+                        )
+                    ) {
+                        "At targeting FIELD expects its opcode to be one of: GETFIELD, GETSTATIC, PUTFIELD, PUTSTATIC"
+                    }
+                    val isGet = opcode == Opcodes.GETFIELD || opcode == Opcodes.GETSTATIC
+                    val isStatic = opcode == Opcodes.GETSTATIC || opcode == Opcodes.PUTSTATIC
+                    FieldTarget(Descriptor.Parser(target).parseField(full = true), isGet, isStatic)
+                } else {
+                    FieldTarget(Descriptor.Parser(target).parseField(full = true), null, null)
+                }
+            }
+            "CONSTANT" -> {
+                require(args != null) {
+                    "At targeting CONSTANT requires args"
+                }
+                args.firstNotNullOfOrNull {
+                    val key = it.substringBefore('=')
+                    val type = when (key) {
+                        "null" -> Any::class.descriptor() // Is this right?
+                        "intValue" -> Descriptor.Primitive.INT
+                        "floatValue" -> Descriptor.Primitive.FLOAT
+                        "longValue" -> Descriptor.Primitive.LONG
+                        "doubleValue" -> Descriptor.Primitive.DOUBLE
+                        "stringValue" -> String::class.descriptor()
+                        "classValue" -> Descriptor.Object("L${it.substringAfter("=")};")
+                        else -> return@firstNotNullOfOrNull null
+                    }
+
+                    ConstantTarget(key, type)
+                } ?: error("At targeting CONSTANT expects a typeValue arg")
+            }
+            else -> error("Invalid At.value for Utils.getAtTarget: ${value}")
+        }
+    }
+
     enum class Shift {
         NONE,
         BEFORE,

src/main/kotlin/com/chattriggers/ctjs/internal/launch/generation/DynamicMixinGenerator.kt

@@ -10,11 +10,8 @@ import java.io.File
 import org.spongepowered.asm.mixin.Mixin as SPMixin
 
 internal class DynamicMixinGenerator(private val ctx: GenerationContext, private val details: MixinDetails) {
-    val generatedClassName = "CTMixin_\$${ctx.mixin.target.replace('.', '_')}\$_${mixinCounter++}"
-    val generatedClassFullPath = "${DynamicMixinManager.GENERATED_PACKAGE}/$generatedClassName"
-
     fun generate(): ByteArray {
-        val mixinClassNode = assembleClass(public, generatedClassFullPath, version = Opcodes.V17) {
+        val mixinClassNode = assembleClass(public, ctx.generatedClassFullPath, version = Opcodes.V17) {
             for ((id, injector) in details.injectors) {
                 when (injector) {
                     is Inject -> InjectGenerator(ctx, id, injector).generate()
@@ -48,13 +45,9 @@ internal class DynamicMixinGenerator(private val ctx: GenerationContext, private
         if (CTJS.isDevelopment) {
             val dir = File(CTJS.configLocation, "ChatTriggers/mixin-classes")
             dir.mkdirs()
-            File(dir, "$generatedClassName.class").writeBytes(bytes)
+            File(dir, "${ctx.generatedClassName}.class").writeBytes(bytes)
         }
 
         return bytes
     }
-
-    companion object {
-        private var mixinCounter = 0
-    }
 }

src/main/kotlin/com/chattriggers/ctjs/internal/launch/generation/GenerationContext.kt

@@ -2,14 +2,21 @@ package com.chattriggers.ctjs.internal.launch.generation
 
 import com.chattriggers.ctjs.api.Mappings
 import com.chattriggers.ctjs.internal.launch.Descriptor
+import com.chattriggers.ctjs.internal.launch.DynamicMixinManager
 import com.chattriggers.ctjs.internal.launch.Mixin
 import org.spongepowered.asm.mixin.transformer.ClassInfo
 
 internal data class GenerationContext(val mixin: Mixin) {
     val mappedClass = Mappings.getMappedClass(mixin.target) ?: error("Unknown class name ${mixin.target}")
+    val generatedClassName = "CTMixin_\$${mixin.target.replace('.', '_')}\$_${mixinCounter++}"
+    val generatedClassFullPath = "${DynamicMixinManager.GENERATED_PACKAGE}/$generatedClassName"
 
     fun findMethod(method: String): Pair<Mappings.MappedMethod, ClassInfo.Method> {
         val descriptor = Descriptor.Parser(method).parseMethod(full = false)
         return Utils.findMethod(mappedClass, descriptor)
     }
+
+    companion object {
+        private var mixinCounter = 0
+    }
 }

src/main/kotlin/com/chattriggers/ctjs/internal/launch/generation/InjectGenerator.kt

@@ -1,5 +1,9 @@
 package com.chattriggers.ctjs.internal.launch.generation
 
+import codes.som.koffee.MethodAssembly
+import codes.som.koffee.insns.jvm.aconst_null
+import codes.som.koffee.insns.jvm.areturn
+import codes.som.koffee.insns.jvm.ldc
 import com.chattriggers.ctjs.internal.launch.Descriptor
 import com.chattriggers.ctjs.internal.launch.Inject
 import com.chattriggers.ctjs.internal.utils.descriptor
@@ -61,4 +65,10 @@ internal class InjectGenerator(
             visitEnd()
         }
     }
+
+    context(MethodAssembly)
+    override fun generateNotAttachedBehavior() {
+        // This method is expected to leave something on the stack
+        aconst_null
+    }
 }