redex icon indicating copy to clipboard operation
redex copied to clipboard

IRTypeChecker false negative when using INVOKE_SUPER on interface default method

Open justhecuke opened this issue 3 years ago • 4 comments

It seems that IRTypeChecker will trigger on an INVOKE_SUPER when called legally on an interface method with a default implementation.

I don't have a good minimal reproduction, but it should be relatively easy to make if you modify an existing application to use Animator.AnimatorListener.onAnimationEnd(Animator, boolean) https://developer.android.com/reference/android/animation/Animator.AnimatorListener#onAnimationEnd(android.animation.Animator,%20boolean).

I'm using a modified repository merged at 6eb187dd4158a6891f857ffd3f0da17efe071553 so it's difficult to reproduce with a head version of this repo.

Here's a redacted error message. I've removed the bytecode print since it's mostly useless as it's only the INVOKE_SUPER that is important here.

Running IRTypeChecker...
libc++abi: terminating with uncaught exception of type boost::exception_detail::error_info_injector<RedexException>: libredex/PassManager.cpp:317: boost::optional<std::string> (anonymous namespace)::CheckerConfig::run_verifier(const Scope &, bool): assertion `!exit_on_fail' failed.
Inconsistency found in Dex code for La$b;.onAnimationEnd:(Landroid/animation/Animator;Z)V
 1. Type error in method La$1;.onAnimationEnd:(Landroid/animation/Animator;Z)V at instruction 'INVOKE_SUPER v4, v5, v6, Landroid/animation/Animator$AnimatorListener;.onAnimationEnd:(Landroid/animation/Animator;Z)V' @ 0x6001812b5e20 for 
illegal invoke-super to interface method defined in class Landroid/animation/Animator$AnimatorListener;(note that this can happen when external framework SDKs are not passed to D8 as a classpath dependency; in such cases D8 may silently generate illegal invoke-supers to interface methods)

justhecuke avatar May 06 '22 21:05 justhecuke

Would you be interested in submitting a patch? https://github.com/facebook/redex/blob/main/libredex/IRTypeChecker.cpp#L735

thezhangwei avatar May 06 '22 21:05 thezhangwei

The dex spec (https://source.android.com/devices/tech/dalvik/dalvik-bytecode) says:

[…] invoke-super […] In Dex files prior to version 037, having an interface method_id is illegal and undefined.

Redex does not fully support version 037. Various Redex optimization passes will silently do the wrong thing when given interface methods on invoke-super instructions, so we added this explicit check in the IRTypeChecker to stop that from happening. There's no easy true fix in Redex here, we'd need to audit and upgrade all passes.

Instead, the error message points out what likely happened, and where to fix this:

note that this can happen when external framework SDKs are not passed to D8 as a classpath dependency; in such cases D8 may silently generate illegal invoke-supers to interface methods

NTillmann avatar May 06 '22 22:05 NTillmann

So this is mostly WAI due to the difficulty of updating for 037+?

Given that the IRTypeChecker will trigger on legal code, I'd suggest updating the error message to indicate non-compatibility with 037+ features, of which calling super into a default interface method is one of them.

justhecuke avatar May 06 '22 23:05 justhecuke

I'm happy to provide a patch, but I'm not certain what scope of fix you're going to accept. Locally, I'm planning on disabling the check to focus on other merge/integration issues that might be hiding behind this until it gets resolved. I can't have my company stop usage of interface default methods, especially since it's part of the official Android API.

justhecuke avatar May 06 '22 23:05 justhecuke