Mixin icon indicating copy to clipboard operation
Mixin copied to clipboard

Mixin seems to be sending null descriptors to method remappers for some deobf cases

Open liach opened this issue 4 years ago • 3 comments

See https://github.com/FabricMC/fabric-loader/issues/397

MixinIntermediaryDevRemapper.mapMethodName doesn't seem to handle a null Mixin method target descriptor correctly when the owner class isn't mapped, causing a NullPointerException. See crash log: crash-2021-03-06_01.27.35-client.txt

This is triggered by the mixin @Redirect(method = "performBuild", at = @At(value = "INVOKE", target = "Lme/jellysquid/mods/sodium/client/render/pipeline/context/ChunkRenderContext;renderBlock"), remap = false) in https://github.com/comp500/Indium/blob/7e734bdc90357f132d7a038048e19020537179c7/src/main/java/link/infra/indium/mixin/sodium/MixinChunkRenderRebuildTask.java#L55

Relevant portion:

Caused by: java.lang.NullPointerException: Cannot invoke "String.hashCode()" because "this.desc" is null
	at net.fabricmc.mappings.EntryTriple.hashCode(EntryTriple.java:67)
	at java.base/java.util.HashMap.hash(HashMap.java:340)
	at java.base/java.util.HashMap.getOrDefault(HashMap.java:1144)
	at net.fabricmc.mapping.util.MixinRemapper.mapMethodName(MixinRemapper.java:66)
	at net.fabricmc.loader.util.mappings.MixinIntermediaryDevRemapper.mapMethodNameInner(MixinIntermediaryDevRemapper.java:74)
	at net.fabricmc.loader.util.mappings.MixinIntermediaryDevRemapper.mapMethodName(MixinIntermediaryDevRemapper.java:139)
	at org.spongepowered.asm.obfuscation.RemapperChain.mapMethodName(RemapperChain.java:59)
	at org.spongepowered.asm.mixin.refmap.RemappingReferenceMapper.remapWithContext(RemappingReferenceMapper.java:164)
	at org.spongepowered.asm.mixin.refmap.RemappingReferenceMapper.remap(RemappingReferenceMapper.java:121)
	at org.spongepowered.asm.mixin.injection.struct.MemberInfo.parse(MemberInfo.java:692)
	at org.spongepowered.asm.mixin.injection.selectors.TargetSelector.parse(TargetSelector.java:154)
	at org.spongepowered.asm.mixin.injection.selectors.TargetSelector.parseAndValidate(TargetSelector.java:109)
	at org.spongepowered.asm.mixin.injection.struct.InjectionPointData.getTarget(InjectionPointData.java:273)
	at org.spongepowered.asm.mixin.injection.points.BeforeInvoke.<init>(BeforeInvoke.java:126)

This should at least be documented in the javadoc.

liach avatar Jun 24 '21 04:06 liach

It is a little ambiguous how lax targets can be, Mixin lets you do more combinations than it is able to remap correctly, but it's not obvious whether that's because remapping might not produce the correct results or because it's broken

Chocohead avatar Jun 30 '21 23:06 Chocohead

So I just stumbled upon the same issue as comp500/Indium#127 and I was just wondering if there is any progress being made on this? It sucks not being able to use Sodium and Indium because my own mod relies on the Fabric Rendering API

Wexalian avatar Jul 05 '22 00:07 Wexalian

At.target Javadoc:

must be specified as a fully-qualified member path including the class name and signature

So more a case of not rejecting the invalid target cleanly, the root cause is on the mod side though.

Player3324 avatar Jul 05 '22 05:07 Player3324