Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IRTypeChecker false negative when using INVOKE_SUPER on interface default method #695

Open
justhecuke opened this issue May 6, 2022 · 4 comments

Comments

@justhecuke
Copy link

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 6eb187d 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)
@thezhangwei
Copy link
Contributor

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

@NTillmann
Copy link
Contributor

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

@justhecuke
Copy link
Author

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants