Guard the hook dispatch against re-entering itself - #803
Draft
JingMatrix wants to merge 1 commit into
Draft
Conversation
JingMatrix
force-pushed
the
hooks-object-getclass
branch
3 times, most recently
from
July 30, 2026 06:17
b98fca2 to
8336afa
Compare
Hooking Object.getClass() made the dispatch call itself. R8 compiles Kotlin's parameter null checks into obj.getClass(), and one of those is the first instruction of the trampoline callback, so the first getClass the process ran after the hook landed re-entered the trampoline, and re-entered again from its own prologue, until the stack was gone and before any hooker had run. Nothing recovers from there: lsplant marks a hooked method non-compilable, so the call site can never be inlined away afterwards, and the framework dex is loaded from memory and never gets an oat file, so the interpreter reaches the trampoline every time. The lowering is new. Built from the same source, 3046 has 16 Object.getClass() call sites in the framework dex and 3047 has 207; the difference between them is #792, which moved the build from AGP 8.13.1 to 9.3.1. Reported as #798, where a module that hooks every method named getClass over Class.getMethods() - a list that always contains the one inherited from Object - crash-looped com.miui.home through six process starts. The build it was compared against turned out to be a branch artefact from before the migration; the version code counts commits on master rather than on the branch being built, which is why it read as 3047. The trampoline entry point is native now, so the re-entrancy check runs ahead of anything a compiler can put in front of it. A Kotlin body cannot promise that, which is the whole of the bug. While the guard is raised, a hooked method entered from the framework's own frames runs its original instead of dispatching again. Where the guard comes down took several attempts on a device to get right, and the shape that finally works comes from writing VectorChain in Java. The chain is the surface the API hands to modules, so every method on it is entered from module code with the guard down, and the guard cannot cover a callee's prologue - in Kotlin, R8 opens each with a parameter null check compiled into Object.getClass(), a hookable call ahead of any statement of ours. A hooker that rebuilds its arguments then re-enters twice per frame and the nesting cap bounds a tree rather than a chain. The parameter types come from a Java @nonnull interface and cannot be made nullable, so the only way not to emit the checks is not to write this in Kotlin; javac emits none. Being Java, the chain's own bookkeeping calls nothing a module can hook - a constructor, an array read, two field writes. So it needs no guard of its own: one lower per node covers both calls that leave the framework, the hooker and the terminal, and nothing raises. The guard comes down for those two, and for the original run through an Invoker; it is raised only by the native trampoline and, for its own bookkeeping, by the legacy bridge. Because a site that gets this wrong is invisible until a device hangs, the two sides live in DispatchGuard.kt as callIntoModule and enterFramework, the raw primitives are named nowhere else, and checkDispatchGuard fails the build if they are. That check is not sufficient on its own: the Java-facing wrappers in that same file were at one point recursing into themselves through a SAM conversion, which reads correctly in source and only shows in the bytecode. A hooker that calls the method it hooks recurses in module code, where the guard does not reach, and hooking Object.getClass has the compiler write such calls on the module's behalf. Past a nesting of thirty-two the thread latches into serving originals until it unwinds, and names the method once. Latching rather than re-arming per frame matters for the same reason as above: a hooker that re-enters more than once per frame would otherwise branch at every level. Three things follow from the entry point being native. A registration that fails now refuses the hook, because the alternative is UnsatisfiedLinkError thrown out of whatever the application was calling, which is far harder to trace back. The two invoke bridges take the argument array rather than a vararg, since the spread copied it on every dispatch and the JVM descriptor is the same either way. And the trampoline's package joins the ones the daemon renames as it loads the dex: keeping a native method keeps its class name, so R8 stopped renaming it, and it would otherwise stand as a fixed string in every injected process. Verified on a Pixel 7a running Android 16, against builds of this branch and of master without it. Without: the target hangs on the first dispatch after the hook lands and is killed by an ANR whose trace holds no main thread, which is the signature reported in #798. With, and with that hook live throughout: 33 results, one process id, no ANR, and the cap firing once, for the hooker written to recurse into itself. That covers ordinary, static and constructor hooks, a class initializer, both Invoker types, a hooker that rebuilds its arguments, a legacy de.robv hook, a hooker seeing another module's hook, and an Invoker whose original calls a method hooked elsewhere. An empty pass-through hook measured about ten percent slower per dispatch than master, from the added native round trip. Fixes #798.
JingMatrix
force-pushed
the
hooks-object-getclass
branch
2 times, most recently
from
July 30, 2026 06:48
1c356a1 to
72863a6
Compare
JingMatrix
marked this pull request as draft
July 30, 2026 07:34
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Since #792, R8 compiles Kotlin's parameter null checks into
obj.getClass(), and one of them is thefirst instruction of
VectorNativeHooker.callback. A module hookingObject.getClassthereforemakes the dispatch re-enter itself from its own prologue. 3046 has 16 such call sites in the
framework dex; 3047 has 207.
The trampoline entry point is a native method now, so nothing precedes the re-entrancy check. The
guard comes down for the two calls that leave the framework — a hooker, and the original method.
The chain is where that has to hold, and it is written in Java for the reason: every method on it is
entered from module code, and in Kotlin R8 would open each with a null check compiled into
Object.getClass(), a hookable call ahead of any statement of ours.@NonNullJava parameterscannot be made nullable, so the checks can only be avoided by not writing it in Kotlin. Being Java,
its bookkeeping calls nothing a module can hook, so a whole node is one guard-lower and no raise.
Getting a guard site wrong still returns correct results and only shows as a hang under load, so
both sides are
callIntoModule/enterFrameworkinDispatchGuard.kt, the primitives are namednowhere else, and
checkDispatchGuardfails the build if they are — not sufficient on its own,since the Java-facing wrappers there were at one point recursing through a SAM conversion, which
reads correctly in source.
A hooker calling the method it hooks is out of the guard's reach; past a nesting of 32 the thread
latches into serving originals until it unwinds, and names the method once.
Also: a failed
RegisterNativesrefuses the hook rather than throwingUnsatisfiedLinkErrorat theapp; the invoke bridges take an array rather than a vararg; the trampoline's package joins the ones
the daemon renames, since a native method keeps its class name under R8.
Verified on a Pixel 7a / Android 16 with an adb-driven two-app harness, against this branch and
master. Master ANRs on the first dispatch after
Object.getClassis hooked, with nomainthreadin the trace — the #798 signature. This branch keeps that hook live throughout: 33 results, one pid,
no ANR, the nesting cap silent except for a hooker written to recurse into itself. Covered:
ordinary/static/constructor hooks, a class initializer, both Invoker types, a hooker that rebuilds
its arguments, a legacy
de.robvhook, a hooker seeing another module's hook, and an Invoker whoseoriginal calls a method hooked elsewhere. An empty pass-through hook is ~10% slower per dispatch than
master, from the added native round trip.
Fixes #798.