SpongePowered / Mixin

Mixin is a trait/mixin and bytecode weaving framework for Java using ASM
MIT License
1.37k stars 185 forks source link

Initializer merging behavior seems to have changed in 0.8.6 snapshots #658

Closed zml2008 closed 2 months ago

zml2008 commented 6 months ago

I'm afraid I've been a bit checked out of things for a while, but just poking around the the 0.8.6 snapshots on Sponge (see SpongePowered/Sponge#3937), I'm getting a new runtime failure based on the same Mixins and target classes. Looking at the exported transformed class file, the initializer merging isn't matching what I'd expect.

The Mixin in question

Stacktrace ``` [19:50:01] [main/ERROR]: Failed to invoke main class org.spongepowered.vanilla.applaunch.Main due to an error: Caused by: java.lang.ExceptionInInitializerError at net.minecraft.server.Bootstrap.bootStrap(Bootstrap.java:50) at org.spongepowered.vanilla.launch.IntegrationTestLaunch.performIntegrationTests(IntegrationTestLaunch.java:70) at org.spongepowered.vanilla.launch.VanillaBootstrap.perform(VanillaBootstrap.java:64) at org.spongepowered.vanilla.launch.IntegrationTestLaunch.performBootstrap(IntegrationTestLaunch.java:63) at org.spongepowered.vanilla.launch.VanillaLaunch.launchPlatform(VanillaLaunch.java:118) at org.spongepowered.vanilla.launch.IntegrationTestLaunch.launch(IntegrationTestLaunch.java:49) ... 23 more Caused by: java.lang.NullPointerException: Cannot invoke "java.util.Map.put(Object, Object)" because "this.impl$entries" is null at net.minecraft.core.MappedRegistry.bridge$register(MappedRegistry.java:1100) at net.minecraft.core.MappedRegistry.handler$zef000$impl$cacheRegistryEntry(MappedRegistry.java:1089) at net.minecraft.core.MappedRegistry.registerMapping(MappedRegistry.java:180) at net.minecraft.core.MappedRegistry.register(MappedRegistry.java:185) at net.minecraft.core.registries.BuiltInRegistries.internalRegister(BuiltInRegistries.java:221) at net.minecraft.core.registries.BuiltInRegistries.registerDefaultedWithIntrusiveHolders(BuiltInRegistries.java:213) at net.minecraft.core.registries.BuiltInRegistries.registerDefaultedWithIntrusiveHolders(BuiltInRegistries.java:201) at net.minecraft.core.registries.BuiltInRegistries.(BuiltInRegistries.java:119) ... 29 more ```

impl$entries is a field added by the Mixin with an initializer, so I wouldn't expect it to be null here. Looking at the dump, I get the following decompile output (comments mine) -- it seems the jump checking $$2 is jumping to right after those initializers:

  public MappedRegistry(ResourceKey<? extends net.minecraft.core.Registry<T>> $$0, Lifecycle $$1, boolean $$2) {
   /* [.. trimmed original target class initializer */
    if ($$2) {
      this.unregisteredIntrusiveHolders = new IdentityHashMap();
      /* our transformations start here */
      this.impl$entries = new LinkedHashMap();
      this.impl$isDynamic = true;
    }

    this.handler$zef000$impl$setType($$0, $$1, $$2, new CallbackInfo("<init>", false));
  }
Method bytecode ```java // access flags 0x1 // signature (Lnet/minecraft/resources/ResourceKey<+Lnet/minecraft/core/Registry;>;Lcom/mojang/serialization/Lifecycle;Z)V // declaration: void (net.minecraft.resources.ResourceKey>, com.mojang.serialization.Lifecycle, boolean) public (Lnet/minecraft/resources/ResourceKey;Lcom/mojang/serialization/Lifecycle;Z)V L0 LINENUMBER 90 L0 ALOAD 0 INVOKESPECIAL java/lang/Object. ()V L1 LINENUMBER 40 L1 ALOAD 0 NEW it/unimi/dsi/fastutil/objects/ObjectArrayList DUP SIPUSH 256 INVOKESPECIAL it/unimi/dsi/fastutil/objects/ObjectArrayList. (I)V PUTFIELD net/minecraft/core/MappedRegistry.byId : Lit/unimi/dsi/fastutil/objects/ObjectList; L2 LINENUMBER 40 L2 ALOAD 0 NEW it/unimi/dsi/fastutil/objects/Object2IntOpenCustomHashMap DUP INVOKESTATIC net/minecraft/Util.identityStrategy ()Lit/unimi/dsi/fastutil/Hash$Strategy; INVOKESPECIAL it/unimi/dsi/fastutil/objects/Object2IntOpenCustomHashMap. (Lit/unimi/dsi/fastutil/Hash$Strategy;)V INVOKEDYNAMIC accept()Ljava/util/function/Consumer; [ // handle kind 0x6 : INVOKESTATIC java/lang/invoke/LambdaMetafactory.metafactory(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodHandle;Ljava/lang/invoke/MethodType;)Ljava/lang/invoke/CallSite; // arguments: (Ljava/lang/Object;)V, // handle kind 0x6 : INVOKESTATIC net/minecraft/core/MappedRegistry.lambda$new$0(Lit/unimi/dsi/fastutil/objects/Object2IntOpenCustomHashMap;)V, (Lit/unimi/dsi/fastutil/objects/Object2IntOpenCustomHashMap;)V ] INVOKESTATIC net/minecraft/Util.make (Ljava/lang/Object;Ljava/util/function/Consumer;)Ljava/lang/Object; CHECKCAST it/unimi/dsi/fastutil/objects/Object2IntMap PUTFIELD net/minecraft/core/MappedRegistry.toId : Lit/unimi/dsi/fastutil/objects/Object2IntMap; L3 LINENUMBER 56 L3 ALOAD 0 NEW java/util/HashMap DUP INVOKESPECIAL java/util/HashMap. ()V PUTFIELD net/minecraft/core/MappedRegistry.byLocation : Ljava/util/Map; L4 LINENUMBER 56 L4 ALOAD 0 NEW java/util/HashMap DUP INVOKESPECIAL java/util/HashMap. ()V PUTFIELD net/minecraft/core/MappedRegistry.byKey : Ljava/util/Map; L5 LINENUMBER 56 L5 ALOAD 0 NEW java/util/IdentityHashMap DUP INVOKESPECIAL java/util/IdentityHashMap. ()V PUTFIELD net/minecraft/core/MappedRegistry.byValue : Ljava/util/Map; L6 LINENUMBER 56 L6 ALOAD 0 NEW java/util/IdentityHashMap DUP INVOKESPECIAL java/util/IdentityHashMap. ()V PUTFIELD net/minecraft/core/MappedRegistry.lifecycles : Ljava/util/Map; L7 LINENUMBER 56 L7 ALOAD 0 NEW java/util/IdentityHashMap DUP INVOKESPECIAL java/util/IdentityHashMap. ()V PUTFIELD net/minecraft/core/MappedRegistry.tags : Ljava/util/Map; L8 LINENUMBER 56 L8 ALOAD 0 NEW net/minecraft/core/MappedRegistry$1 DUP ALOAD 0 INVOKESPECIAL net/minecraft/core/MappedRegistry$1. (Lnet/minecraft/core/MappedRegistry;)V PUTFIELD net/minecraft/core/MappedRegistry.lookup : Lnet/minecraft/core/HolderLookup$RegistryLookup; L9 LINENUMBER 90 L9 ALOAD 1 INVOKEDYNAMIC get(Lnet/minecraft/resources/ResourceKey;)Ljava/util/function/Supplier; [ // handle kind 0x6 : INVOKESTATIC java/lang/invoke/LambdaMetafactory.metafactory(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodHandle;Ljava/lang/invoke/MethodType;)Ljava/lang/invoke/CallSite; // arguments: ()Ljava/lang/Object;, // handle kind 0x6 : INVOKESTATIC net/minecraft/core/MappedRegistry.lambda$new$1(Lnet/minecraft/resources/ResourceKey;)Ljava/lang/String;, ()Ljava/lang/String; ] INVOKESTATIC net/minecraft/server/Bootstrap.checkBootstrapCalled (Ljava/util/function/Supplier;)V L10 LINENUMBER 91 L10 ALOAD 0 ALOAD 1 PUTFIELD net/minecraft/core/MappedRegistry.key : Lnet/minecraft/resources/ResourceKey; L11 LINENUMBER 92 L11 ALOAD 0 ALOAD 2 PUTFIELD net/minecraft/core/MappedRegistry.registryLifecycle : Lcom/mojang/serialization/Lifecycle; L12 LINENUMBER 93 L12 ILOAD 3 IFEQ L13 L14 LINENUMBER 94 L14 ALOAD 0 NEW java/util/IdentityHashMap DUP INVOKESPECIAL java/util/IdentityHashMap. ()V PUTFIELD net/minecraft/core/MappedRegistry.unregisteredIntrusiveHolders : Ljava/util/Map; ALOAD 0 NEW java/util/LinkedHashMap DUP INVOKESPECIAL java/util/LinkedHashMap. ()V PUTFIELD net/minecraft/core/MappedRegistry.impl$entries : Ljava/util/Map; ALOAD 0 ICONST_1 PUTFIELD net/minecraft/core/MappedRegistry.impl$isDynamic : Z L13 LINENUMBER 96 L13 FRAME FULL [net/minecraft/core/MappedRegistry net/minecraft/resources/ResourceKey com/mojang/serialization/Lifecycle I] [] ALOAD 0 ALOAD 1 ALOAD 2 ILOAD 3 NEW org/spongepowered/asm/mixin/injection/callback/CallbackInfo DUP LDC "" ICONST_0 INVOKESPECIAL org/spongepowered/asm/mixin/injection/callback/CallbackInfo. (Ljava/lang/String;Z)V INVOKESPECIAL net/minecraft/core/MappedRegistry.handler$zef000$impl$setType (Lnet/minecraft/resources/ResourceKey;Lcom/mojang/serialization/Lifecycle;ZLorg/spongepowered/asm/mixin/injection/callback/CallbackInfo;)V RETURN L15 LOCALVARIABLE this Lnet/minecraft/core/MappedRegistry; L0 L15 0 // signature Lnet/minecraft/core/MappedRegistry; // declaration: this extends net.minecraft.core.MappedRegistry LOCALVARIABLE $$0 Lnet/minecraft/resources/ResourceKey; L0 L15 1 // signature Lnet/minecraft/resources/ResourceKey<+Lnet/minecraft/core/Registry;>; // declaration: $$0 extends net.minecraft.resources.ResourceKey> LOCALVARIABLE $$1 Lcom/mojang/serialization/Lifecycle; L0 L15 2 LOCALVARIABLE $$2 Z L0 L15 3 MAXSTACK = 8 MAXLOCALS = 4 ```
Mumfrey commented 6 months ago

Thanks for this, I have changed the structure of how initialisers are handled to pull functionality out of the applicator and into the target so that information can be shared with CTOR_HEAD, but it's likely this has introduced this issue. I'll have a look into it. This was the entire point of releasing a snaphot first.