Backups/OpenZen
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(patchify): guard HEAD inject arity and correct Embeddium patch docs

Browse Source 
Follow-up hardening for the Embeddium/Sodium XRay compat patch (#54).

- PatchTransformer.injectHead now validates a handler's parameter list
  against the forwarded (receiver, args..., CallbackInfo) sequence before
  emitting bytecode. Name-only (desc="") patches match by method name
  alone, so a handler could bind to a target with a different arity or
  primitive parameter types and produce a VerifyError at class load. Such
  mismatches are now logged and skipped instead.
- Name-only matching in apply() now resolves the target method name via
  getHandlerTargetMethodName(), covering all handler kinds instead of only
  Inject/Overwrite.
- BlockOcclusionCachePatch javadoc: drop the stale claim that registration
  is guarded by Class.forName() (it is registered unconditionally, and must
  be), and clarify that Object params only protect against type mismatches,
  not arity mismatches.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Shirona1337
2026-06-20 00:46:32 +08:00
parent b76ba33ea5
commit f59d4c121c
2 changed files with 75 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/main/java/asm/patchify/loader/PatchTransformer.java
+56 -6
View File
@@ -95,13 +95,12 @@ public final class PatchTransformer {
MethodKey key = new MethodKey(method.name, method.desc);
List<Method> handlers = handlersByTarget.get(key);
if (handlers == null && !nameOnlyHandlers.isEmpty()) {
// Try name-only match for handlers registered with empty desc
// Try name-only match for handlers registered with empty desc.
// Works for every handler kind (Inject/Overwrite/Transform/WrapInvoke/
// ModifyLocals), not just Inject/Overwrite.
for (Method candidate : nameOnlyHandlers) {
Inject inject = candidate.getAnnotation(Inject.class);
Overwrite overwrite = candidate.getAnnotation(Overwrite.class);
String targetName = inject != null ? inject.method()
: overwrite != null ? overwrite.method() : null;
if (targetName != null && targetName.equals(method.name)) {
String targetName = getHandlerTargetMethodName(candidate);
if (targetName.equals(method.name)) {
if (handlers == null) handlers = new ArrayList<>();
handlers.add(candidate);
nameOnlyMatched.add(method.name);
@@ -267,7 +266,58 @@ public final class PatchTransformer {
}
}
/**
* Validates that a HEAD-inject handler's parameter list is bytecode-compatible with the
* {@code (receiver?, args..., CallbackInfo)} sequence that {@link #injectHead} forwards.
*
* <p>This matters most for name-only ({@code desc=""}) patches: those match a target by
* method name alone, so a handler can end up bound to a method whose arity or primitive
* parameter types differ from what it declares. Forwarding into such a handler would emit a
* call site that fails JVM verification ({@code VerifyError}) at class load. Returns
* {@code null} when compatible, otherwise a human-readable reason.</p>
*/
private static String headHandlerMismatch(MethodNode method, Method handler) {
List<Type> forwarded = new ArrayList<>();
if (!Modifier.isStatic(method.access)) {
// Receiver is always a reference; only its primitive-ness is checked below.
forwarded.add(Type.getObjectType("java/lang/Object"));
}
forwarded.addAll(Arrays.asList(Type.getArgumentTypes(method.desc)));
// The last handler parameter is CallbackInfo (enforced by validateInjectSignature);
// the remaining params receive the forwarded receiver + args.
int expected = handler.getParameterCount() - 1;
if (forwarded.size() != expected) {
return "handler takes " + expected + " forwarded parameter(s) but target supplies "
+ forwarded.size() + " (receiver + args)";
}
Class<?>[] params = handler.getParameterTypes();
for (int i = 0; i < forwarded.size(); i++) {
Type arg = forwarded.get(i);
Class<?> p = params[i];
boolean argIsPrimitive = arg.getSort() != Type.OBJECT && arg.getSort() != Type.ARRAY;
if (argIsPrimitive) {
// Primitives cannot widen to Object — the handler param must be the exact type.
if (!p.isPrimitive() || !p.getName().equals(arg.getClassName())) {
return "parameter " + i + " is primitive " + arg.getClassName()
+ " but handler declares " + p.getName();
}
} else if (p.isPrimitive()) {
// A reference arg cannot be passed into a primitive parameter.
return "parameter " + i + " is a reference but handler declares primitive " + p.getName();
}
}
return null;
}
private static void injectHead(MethodNode method, Method handler) {
String mismatch = headHandlerMismatch(method, handler);
if (mismatch != null) {
LOGGER.warn("@Inject(HEAD) {}#{} is incompatible with matched target {}{} — {} — skipping injection to avoid invalid bytecode.",
handler.getDeclaringClass().getName(), handler.getName(),
method.name, method.desc, mismatch);
return;
}
Type returnType = Type.getReturnType(method.desc);
String handlerOwner = Type.getInternalName(handler.getDeclaringClass());
String handlerName = handler.getName();
src/main/java/shit/zen/patch/BlockOcclusionCachePatch.java
+19 -13
View File
@@ -22,12 +22,14 @@ import shit.zen.modules.impl.render.XRay;
*
* <p>The target class is referenced by {@link Patch#className()} rather than by
* {@link Patch#value()} because Embeddium is an optional mod — the class is not
* available at compile time. Registration in
* {@link shit.zen.ZenClient#registerPatches()} is guarded by a
* {@link Class#forName(String)} check so the patch is only loaded when Embeddium is
* present. The method descriptor is left empty (name-only match) because the parameter
* types in Embeddium's bytecode may differ between Yarn and Mojmap mappings depending
* on the Embeddium build and compatibility layer.</p>
* available at compile time. The patch is registered <em>unconditionally</em> in
* {@link shit.zen.ZenClient#registerPatches()} (we must <em>not</em> probe with
* {@link Class#forName(String)}, which would force the target class to load before our
* transformer is installed and thus defeat the patch). When Embeddium is absent the
* target class simply never loads, so the registered patch is harmless. The method
* descriptor is left empty (name-only match) because the parameter types in Embeddium's
* bytecode may differ between Yarn and Mojmap mappings depending on the Embeddium build
* and compatibility layer.</p>
*/
@Patch(className = "me.jellysquid.mods.sodium.client.render.chunk.compile.pipeline.BlockOcclusionCache")
public class BlockOcclusionCachePatch {
@@ -38,13 +40,17 @@ public class BlockOcclusionCachePatch {
* faces visible = rendered through walls) and non-target blocks return
* {@code false} (no faces visible = completely transparent).
*
* <p>All parameters are declared as {@link Object} rather than their actual types
* because Embeddium may be compiled with either Yarn or Mojmap mappings — using
* {@code Object} avoids a {@code VerifyError} when the handler's descriptor
* doesn't match the runtime descriptor exactly. The only param we actually read
* is {@code selfState}, which is cast to Mojmap {@link BlockState} (always
* correct at runtime since the loaded Minecraft classes are Mojmap-mapped in a
* Forge environment).</p>
* <p>All reference parameters are declared as {@link Object} rather than their actual
* types because Embeddium may be compiled with either Yarn or Mojmap mappings — every
* argument of {@code shouldDrawSide} is a reference type, so declaring them as
* {@code Object} lets the forwarded values widen cleanly regardless of the concrete
* runtime types. Note this only protects against <em>type</em> mismatches: the
* parameter <em>count</em> must still equal the target method's arity (receiver + args),
* otherwise the generated call site is invalid. {@code PatchTransformer.injectHead}
* now guards this and skips the injection with a warning rather than emitting bytecode
* that would fail verification. The only param we actually read is {@code selfState},
* which is cast to Mojmap {@link BlockState} (always correct at runtime since the loaded
* Minecraft classes are Mojmap-mapped in a Forge environment).</p>
*
* @param self the {@code BlockOcclusionCache} instance (unused)
* @param selfState the {@code BlockState} of the block being rendered