apk-reverse/references/patch-audit.md

Auditing your own patch — proving it landed, and proving it is legal

Two independent questions, and they fail independently:

1. Did the edit land? Did the bytes you intended actually reach the artifact?
2. Is the result legal? Will the runtime's verifier accept it?

A patch can land and be illegal (the class fails to load). It can also fail to land while every build step reports success. Both are silent, and each needs its own check.

1. Did it land? Compare length, not bytes

The naive check is a byte-level diff of the target method against the original. It produces a flood of false positives. A disassemble→reassemble round-trip rebuilds the string pool — the assembler de-duplicates identical strings — which shifts every later string index by a small delta. Hundreds of methods then look "changed" while their behaviour is identical.

Observed shape of that noise: ~800 methods reported as differing, 84% of them by only 1–4 bytes, all of them constant-pool index shifts, with the method byte-length unchanged.

Use instruction-stream length as the first-order predicate. A real patch that changes control flow or constant structure changes the method's code_item.insns_size. Pure index shifts and debug-info reordering do not.

The blind spot, and how to close it

An equal-length replacement changes nothing measurable by length.

Examples that bite: move-result vX → const/4 vX, 0x0 (both one code unit); swapping one const for another of the same width; any edit that keeps the instruction count identical.

Length-only auditing therefore silently under-reports: an audit reported 20 changed methods on a build whose patch set actually touched 24 — the four missing ones were all equal-length replacements.

Two-tier rule:

1. Tier 1 (length): scan every method. This yields candidates with essentially zero false positives.
2. Tier 2 (opcode-level): compare the two versions opcode by opcode for every method the patch set declares as a target — not merely the Tier-1 candidates.

Tier 2's input cannot come from Tier 1's output. Equal-length replacements produce no Tier-1 candidate at all, so the only reliable index of "what to check at Tier 2" is the patch set's own declarations.

⇒ Corollary, which is really a documentation requirement: every patch must declare its target method signature. A patch that is both equal-length and undeclared is invisible to both tiers; finding it requires opcode-comparing every class, which is possible but expensive.

A sharper Tier-2 shape for equal-length replacements

Compare the position set of one opcode inside the method:

target method, `move-result` (0x0a) position sets
  original : [25, 43, 66, 72, 216, ... , 899]   (16 entries)
  patched  : [25, 43,     72, 216, ... , 899]   (15 entries)   <- exactly one removed, pc=66

This proves two things at once: the intended site was hit, and the other same-shaped call sites in that method were not. Asking "did anything differ" cannot distinguish those.

2. Is it legal? The verifier is a layer static checks cannot see

An assembler accepting your smali proves the file is syntactically valid dex. It does not run the bytecode verifier. A build can pass assembly, pass every class-table check, and still be rejected at class load:

java.lang.VerifyError: Verifier rejected class X: void X.<clinit>() failed to verify:
  [0x1C8] copyRes1 v0<- result0 type=Undefined

Root cause in the observed case: padding instructions were inserted between a producer and its move-result. DEX requires move-result* to immediately follow the instruction that produces the result (invoke-*, filled-new-array*). The padding broke adjacency, the result type became Undefined, and the whole method — and therefore the class — failed to verify. The length arithmetic ("the shorter form plus two padding units preserves the total") was correct and still produced an illegal method.

Four layers, cheapest first:

Layer	Check	Catches
1	assembler exit code	syntax, register overflow, bad labels
2	class-table diff (scripts/dex_classdiff.py)	class-set / access-flag drift
3	predecessor legality of move-result*	result consumers detached from their producer
4	device: first execution of that method	everything above, plus real semantics

Layer 3 is a set-difference, not an absolute count.

• For both builds, collect every move-result* whose immediately preceding instruction is not a producer → the "offenders" set.
• Compare the sets. New offenders must be 0.
• Never use the absolute size as a signal. A perfectly healthy build can show hundreds of offenders under this definition — payload pseudo-instructions (packed-switch-payload, fill-array-data-payload, …) are normally read as ordinary instructions by a naive linear scan — so the absolute number is dominated by scanner artifacts. Only the delta means anything.

Worked consequence: a build whose sole new offender was one move-result following padding crashed at class load on device. After moving the padding to after the move-result, the delta returned to 0 and the same build ran. The delta predicted the device result before the device was touched.

3. When patches are applied by text matching

Some toolchains apply patches by literal match against a text form (smali) rather than through a dex API. The failure modes are textual and unforgiving:

• Whitespace is content. A fragment indented with 5 spaces will not match a file using 4. There is no fuzzy matching; one misplaced space is a zero-hit.
• Line endings. If the reader normalises CRLF→LF and the writer does not restore it, the patched file becomes LF-only. A later byte-level comparison against the CRLF source then shows the entire file as changed. Diff with an ignore-CR-at-EOL option, or compare structure.
• Instruction names must be complete. move-result vX and move-result-object vX are different instructions; an anchor missing the suffix matches the wrong thing or nothing.
• Operand punctuation matters. instance-of p3, p0, LType is not the source text when the source reads LType;.
• Uniqueness is a hard requirement. A matcher that finds several hits must fail loudly rather than patch the first. When a one-line anchor is not unique, extend it with a second unique line; when the combination is still not unique, add a third anchor that discriminates the homographs — usually the branch target label, since identical call shapes often differ in where they jump.
• A re-emitted old fragment is not a failed write. A patch may legitimately re-emit the old text inside the new one (inserting a guard before the lines it matched). A read-back check asserting "the old text is gone" then reports failure on that patch forever. Assert instead that the new, unique part of the replacement is present.
• One patch document, one patch block. A parser that treats every fenced block as a candidate will apply an example diff as a real patch — silently cancelling the real one, or editing unrelated code. Keep examples in prose, never in the same fence format.

4. Audit the patch set, not a patch

For a multi-patch build:

expected = every method declared as a target by the patch set
tier1    = methods whose insns_size changed
tier2    = methods whose opcode stream differs        (run over `expected`)
missing  = expected - (tier1 U tier2)                 # must be empty

missing is the number that matters. A build can be green on every build step and still be missing a patch. Report missing explicitly; do not report the total as if the total were the goal.

And re-verify the artifact, not the intent. If the tool says "applied" but the build on the device is unchanged, you have a stale build, a second copy, or the wrong file (pitfalls.md P24). A patch tool's success message describes the tool, not the artifact.