java.lang.VerifyError: Bad local variable type when injecting with locals

[stephan-gh]

Injection point: MinecraftServer.getTabCompletions on Minecraft 1.11 (see link for exact injection point).

Trying to inject at the first return with local variable capture results in two different signatures in the IDE and in production:

• IDE: [Ljava/util/List;, Z, [Ljava/lang/String;, Ljava/lang/String;] (correct)
• Production: [Ljava/util/List;, Z, Ljava/util/List;, Ljava/util/Iterator;]
  • This is weird since there is no local variable with a second list at the first return point.

If I add an @Inject and a @Surrogate for both signatures it works fine in the IDE. However, in production, it throws a java.lang.VerifyError: Bad local variable type.

Full error: https://gist.github.com/Minecrell/ca35a3aa503c73265416f92ee8675876

Injector:

@Inject(method = "getTabCompletions", at = @At(value = "RETURN", ordinal = 0), cancellable = true, locals = LocalCapture.CAPTURE_FAILHARD)
private void onTabCompleteChat(ICommandSender sender, String input, BlockPos pos, boolean hasTargetBlock, CallbackInfoReturnable<List<String>> cir,
        List<String> list, boolean b, String[] s1, String s) {
    // ...
}

@Surrogate
private void onTabCompleteChat(ICommandSender sender, String input, BlockPos pos, boolean hasTargetBlock, CallbackInfoReturnable<List<String>> cir,
        List<String> list, boolean b, List<?> list2, Iterator<?> iterator) {
    // ...
}

Is Mixin detecting something wrong here? It seems like the error occurs because Mixin is trying to assign a variable with a wrong type.

[Mumfrey]

Yeah this has reared it's head multiple times in the past for various reasons, and it's usually a function of one of:

• ASM "expand frames" actually expands using a pretty dumb algorthim, though that doesn't look like the case here
• The fact that the decompiling with MCP doesn't just decompile, but also "fixes" a bunch of things (both MCInjector and the Forge Fernflower fix a lot of decompile derp) and those fixes can sometimes rearrange the LVT
• The fact that to correct for these problems, Mixin has to do a lot of reverse-engineering of the code up to the injection point to try to build a coherent picture of what locals are available, but this logic can sometimes be defeated by code Mixin isn't expecting

"Enhanced for" loops in Java are a pretty sticky one because they are normally decompiled to an iterator, so it's actually likely that the production one is "correct", for example if in the original code the two locals astring and s2 don't exist or are initialised inside the foreach loop, but the compiler optimises them outside the loop (putting them later in the locals table but earlier in the code) would be a totally plausible reason for that to happen).

Going to need to look into this in detail, as it's one of the most problematic things I've encountered to date and this looks like yet another corner case to deal with, yay funtimes. With luck it might be fixable by leaving the surrogate in place but just changing the injection logic to use new locals slots instead of reusing them like the current injector does.