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

fix: NPE in unused EcxHandler block removal code (#2086) #2104

Merged
merged 1 commit into from
Feb 15, 2024

Conversation

nitram84
Copy link
Contributor

Description

In a comment to PR #2086 I tried to report a NPE in some cases, but I wasn't able to provide a stable plain java test case. Now I have an apk with the same issue.

Steps to reproduce:

java.lang.NullPointerException: null
	at jadx.core.utils.blocks.BlockSet.get(BlockSet.java:24)
	at jadx.core.dex.visitors.blocks.BlockExceptionHandler.removeUnusedExcHandlers(BlockExceptionHandler.java:603)
	at jadx.core.dex.visitors.blocks.BlockExceptionHandler.process(BlockExceptionHandler.java:72)
	at jadx.core.dex.visitors.blocks.BlockProcessor.independentBlockTreeMod(BlockProcessor.java:325)
	at jadx.core.dex.visitors.blocks.BlockProcessor.processBlocksTree(BlockProcessor.java:51)
	at jadx.core.dex.visitors.blocks.BlockProcessor.visit(BlockProcessor.java:44)
	at jadx.core.dex.visitors.DepthTraversal.visit(DepthTraversal.java:26)
	at jadx.core.dex.visitors.DepthTraversal.lambda$visit$1(DepthTraversal.java:14)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1541)
	at jadx.core.dex.visitors.DepthTraversal.visit(DepthTraversal.java:14)
	at jadx.core.ProcessClass.process(ProcessClass.java:75)
	at jadx.core.ProcessClass.generateCode(ProcessClass.java:118)
	at jadx.core.dex.nodes.ClassNode.decompile(ClassNode.java:391)
	at jadx.core.dex.nodes.ClassNode.getCode(ClassNode.java:336)
	at jadx.api.JadxDecompiler.lambda$appendSourcesSave$2(JadxDecompiler.java:373)
	at jadx.core.utils.tasks.TaskExecutor.wrapTask(TaskExecutor.java:166)
	at jadx.core.utils.tasks.TaskExecutor.lambda$runStages$0(TaskExecutor.java:147)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:829)

After adding a check for null the result looks better. Checking for null shouldn't be wrong, but this fix is probably incomplete, as this won't fix the root cause. Code caches need to be cleared to test this fix.

For android.support.v4.graphics.TypefaceCompatUtil.mmap(File file) it helped too, but compared to the latest release v1.4.7 there is still a regession. This method is very simple and the source code is known (https://android.googlesource.com/platform/frameworks/support/+/a9ac247af2afd4115c3eb6d16c05bc92737d6305/compat/src/main/java/androidx/core/graphics/TypefaceCompatUtil.java#81). Maybe this regression is fixable.

@skylot
Copy link
Owner

skylot commented Feb 15, 2024

Thanks for test case!
Fix is fine, but handler block in exception handler shouldn't be null, so something goes wrong in try/catch processing. I will try to investigate this.
And as soon as I will merge this fix as a temporary solution, @nitram84 can you open a new issue with mentioned test case? 🙂

@skylot skylot merged commit 5fbabde into skylot:master Feb 15, 2024
5 checks passed
skylot added a commit that referenced this pull request Feb 19, 2024
@skylot
Copy link
Owner

skylot commented Feb 19, 2024

Looks like I found cause of unused exception handlers 😢
It was code to remove shadowed handlers (wider typed catch before more narrowed type), but implementation and checks were poor. Also for try with resources, compiler generate weird catch blocks with Throwable and ALL types for same instructions but with different handler code. Removing such handlers is not correct and lead to missing code without warnings, such behavior is unacceptable. So I decided to remove such code and start working on a more general approach to restore try with resources (old but still not resolved issue: #101).

@nitram84 nitram84 deleted the fix_npe_in_pr_2086 branch March 18, 2024 10:25
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

Successfully merging this pull request may close these issues.

2 participants