DexPatcher Tool Roadplan

[Lanchon]

A NOTE REGARDING CHANGES THAT BREAK BACKWARDS COMPATIBILITY: All backwards breaking changes should be implemented in the DexPatcher 'annotations' package but not in the tool itself. It is ok for the source code of an old patch to fail to compile against a newer 'annotations' package: if a developer upgrades the DexPatcher toolchain she might need to bring the patch source code up to date too. But an old patch binary should continue to apply cleanly using newer versions of the DexPatcher tool. This means that, even though the 'annotations' package might have changed the way of doing things, the tool should recognize and honor both ways, old and new, in the patch binaries it operates upon.

• Detect default constructors (that could potentially have been added by the compiler) and if they are trivial (ie, no code besides super call) don't error if there is no action defined for them. Allows not having to define and ignore default constructors (if there are no instance initializers in the class). This is a backwards compatible change. done: v1.3.1.

• Break the direct/virtual method and static/instance field barriers (Breaking the direct/virtual method and static/instance field barriers #19). done: v1.6.0.

• Add @DexCheck that targets items but does not modify them. It checks that they exist, minimum accessibility, and static- and constructor-ness match. This helps ensure backwards compatibility of new apps to old patches.

• Add a class checker that checks contents of a class against the source. The only actions allowed within it are @DexIgnore and @DexCheck. Make @DexReplace and @DexCheck invoke the checker if the item they are processing is a class. Add a defaultAction element to those tags, make it default to @DexIgnore for backwards compatibility.

• In the Gradle plugins, add an option to inhibit the import of the source symbols. This option can be enabled to make sure that every symbol referenced in the patch is also declared in the patch. If they are declared using @DexCheck, then all symbols will be checked at patch time. This is more developer work but makes the patch fail at patch time if applied against a new version of the app that is missing a symbol (instead of producing a patched app that will fail at runtime with link errors). done:
  v0.4.5.

• The above works if @DexIgnore is not used on any symbol. However trivial default constructor auto-ignore makes it difficult to keep track of all ignores at development time. The patch could be invoking a certain trivial default constructor that is being auto-ignored but also exists in the app, and a new version of the app might not provide it, and this would end up in a runtime link error. To ensure this cannot happen, add a command line option to the tool to disable trivial default constructor auto-ignore. Note that all information regarding application of a patch should always be encoded in the patch itself and never in command line options to the tool. But in this case, the option is not needed to apply the patch; it is only a development time aid. This also involves a Gradle plugin change, of course. done: v1.4.1, v0.4.6.

• Add @DexWrap (Implement DexWrap. #9). done: v1.3.0.

• Add @DexAppend (Implement DexAppend. #10) and maybe DexPrepend for symmetry. This is perfect for static constructors but does not really work for instance constructors. done: v1.4.0.

• Change the way static constructors (SC's) are handled when no action is defined for them, from triggering an error to this strategy (this is a backwards compatible change):

  • if there is no SC in the patch, do nothing.
  • if there is one:
    • if there is no SC in the source, do a @DexAdd.
    • if there is one, do a @DexAppend.

  done: v1.4.0.

• Have the new way of handling SC's even if a defaultAction is defined for the class. For this, staticConstructorAction could be decoupled from defaultAction in a backwards breaking change (more on this later), or a DexAction.AUTO value could be added. i decided against introducing this semantic change: it does not pull its own usability weight this late in the game.

  UPDATE: i implemented this by adding a DexAction.NONE element value. it can be used for staticConstructorAction, but the motivation was to better support future @DexCheck-style checking of types being replaced or removed. for backwards compatibility, the replace and remove tags when used on types must not @DexCheck the contents of the patch types they are tagging, effectively making @DexIgnore the default action for them. but a saner behavior where DexPatcher forces you to tag every member explicitly can be restored with @DexReplace/DexRemove(defaultAction = DexAction.NONE). so in the end it is...

  done: v1.6.3.

• Convert staticConstructorAction and defaultAction from an enum type to a nested annotation. This is a backwards breaking change, and a good time to implement the previous change.

• Remove support for the deprecated DexTag and change Void.class to void.class as the default value of the targetClass element. done: v1.7.0.

• Regarding @DexEdit on classes where the patch and targeted classes differ (ie: cross-class edits) on today's DexPatcher: If code from the targeted class is used at all to build the resulting bytecode, the bytecode will almost always be invalid in some ways and will trigger a verification exception (due to differences in the types of the this references, constructors, the hierarchy, etc). Additionally, DexPatcher has had an ingrained golden rule where the bytecode ID of injected items will always match that of the items that trigger the injection in the patch. That way it will always be valid to use those items from the patch code: the item in the patch causing the injection serves as the declaration at patch compile-time for the injected item at runtime. This rule was covertly broken by the second gen tags (@DexWrap, @DexPrepend, @DexAppend) which add hidden items behind-the-scenes, and needs to be overtly broken to fix cross-class edits (and to implement contentOnly, see below). Because of this rule, if onlyEditMembers = true is used on cross-class edits, in reality the name of the class (which is its bytecode ID) is also edited (but not other class attributes) to uphold the rule, even though the unhappy name onlyEditMembers suggests otherwise. This means that for cross-class edits, regardless of the value of onlyEditMembers the name of the class is always changed, invalidating the resulting bytecode; so we have verification exceptions again. To fix cross-class edits, when onlyEditMembers = false DexPatcher has to fully rewrite the targeted class (including code) to replace the type of the target with that of the patch before the edit begins, and when onlyEditMembers = true it has to do the reverse (rewrite the patch class to replace the type of the patch with that of the targeted class before the edit begins) and also fully honor the concept of onlyEditMembers by not renaming the class. This is in theory a backwards breaking change for cross-class edits, but AFAIK these edits never worked except for extremely trivial cases that should continue to work under the new semantics. done: v1.5.0.

• Deprecate the onlyEditMembers element and replace it with contentOnly. This is to allow extending the usage of the element to other tags besides @DexEdit and to other items besides classes. The new tool accepts both the old and new names of the element, so binary patches compiled for an older version of the tool will continue to apply using the new version. However, recompiling the patches will require adapting them to the name change. done: v1.5.0.

• Add a contentOnly element to @DexReplace. @DexEdit and @DexReplace with contentOnly = true would be valid only for classes and would both cause class rewriting (see above) if cross-class. done: v1.5.0.

• Add a contentOnly element to all remaining targeting actions except @DexRemove so that the item injected into the patched code is injected at the targeted location instead of at the patch location, as usual. (This breaks the golden rule mentioned above.) For methods, @DexReplace, @DexWrap, @DexPrepend and @DexAppend would be valid with contentOnly = true and would only replace the existing method's code but not its declaration. For packages, only @DexReplace could be valid with contentOnly = true, but the effort is probably not justified. BTW, regular @DexAppend does not work on instance constructors but could be made to work (with a lot of development effort) by targeting constructors with contentOnly = true from a direct non-static method (ie, a private instance method) any non-static method.

• With @DexCheck in place, allow a patch to be compatible with some changes in the source app:

  • Allow two tags, one targeted and one non-targeted, for each item, such as @DexEdit or @DexIgnore, as alternatives for when the target is found or not.
  • Some method of tallying to verify for example that one and only one of several items are actually targeting must be provided.
  • Special cases such as @DexWrap or @DexAdd must be considered carefully, given that the recursive call in @DexAdd is definitely not what is intended.
  • Syntactic options:
    • Allow more than one tag on the items. But how is tallying handled then?
    • Allow a fallback element on targeting tags of type DexAction (given that non-targeting tags do not currently have elements) or nested annotation. Tallying is possible albeit ugly, via more elements.
    • An annotation such as @DexOption(ifAction=@..., elseAction=@..., id="...", min=1) could take care of tallying-by-id nicely. Id's could be local to the class or global. Nested @DexOption could allow pre-type processing of the patch method to allow the same patch method to apply against similar methods in different version of the source that have changed an argument type.

More to add here:

• General method rewriting.
• Support for obfuscated apps in the Gradle plugins.