Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
liach opened this issue Jun 24, 2021 · 3 comments

Comments

@liach
Copy link

liach commented Jun 24, 2021

See FabricMC/fabric-loader#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.

@Chocohead
Copy link

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

@Wexalian
Copy link

Wexalian commented Jul 5, 2022

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

@sfPlayer1
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants