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

Removing unused ExcHandlers blocks #2086

Merged
merged 5 commits into from
Feb 4, 2024
Merged

Removing unused ExcHandlers blocks #2086

merged 5 commits into from
Feb 4, 2024

Conversation

Away-pp
Copy link
Contributor

@Away-pp Away-pp commented Jan 21, 2024

Removing unused ExcHandlers blocks in order to fix some decompilation errors caused by Unreachable blocks.

The root of this problem (for the current fix) is that some trys may never actually throw an exception.
Therefore the related catch may become unreachable.

The solution is that at the end of the try/catch processing, to look for unreachable blocks from the exception handlers.

An example of method that was not decompiled before:

Original code

https://cs.android.com/android/platform/superproject/main/+/main:frameworks/base/core/java/android/provider/FontsContract.java;l=726?q=prepareFontData

Decompiled code

    private static Map<Uri, ByteBuffer> prepareFontData(Context context, FontInfo[] fonts, CancellationSignal cancellationSignal) {
        HashMap<Uri, ByteBuffer> out = new HashMap<>();
        ContentResolver resolver = context.getContentResolver();
        for (FontInfo font : fonts) {
            if (font.getResultCode() == 0) {
                Uri uri = font.getUri();
                if (!out.containsKey(uri)) {
                    ByteBuffer buffer = null;
                    try {
                        ParcelFileDescriptor pfd = resolver.openFileDescriptor(uri, "r", cancellationSignal);
                        if (pfd != null) {
                            try {
                                FileInputStream fis = new FileInputStream(pfd.getFileDescriptor());
                                try {
                                    FileChannel fileChannel = fis.getChannel();
                                    long size = fileChannel.size();
                                    buffer = fileChannel.map(FileChannel.MapMode.READ_ONLY, 0L, size);
                                    fis.close();
                                } finally {
                                    break;
                                }
                            } catch (IOException e) {
                            } catch (Throwable th) {
                                if (pfd == null) {
                                    throw th;
                                }
                                try {
                                    pfd.close();
                                    throw th;
                                } catch (Throwable th2) {
                                    th.addSuppressed(th2);
                                    throw th;
                                }
                            }
                        }
                        if (pfd != null) {
                            pfd.close();
                        }
                    } catch (IOException e2) {
                    }
                    out.put(uri, buffer);
                }
            }
        }
        return Collections.unmodifiableMap(out);
    }

Copy link
Owner

@skylot skylot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just small requests to improve code.
Also, please add simple Smali test (just copy whole method, also add disableCompilation(), code will not compile because of external classes).

@nitram84
Copy link
Contributor

Here is a minimized standalone example for this issue that could be used for a unit test. Steps to reproduce: compile with openjdk8, "use dx/d8 to convert java bytecode" [x], "Plugins > Java convert > convert mode": "d8"

With convert mode "dx" there is still a NPE.

import java.io.FileInputStream;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.channels.FileChannel;

public class UnusedExceptionHandlers1 implements AutoCloseable {
  public static void doSomething(final Object unused1, final Object[] array, final Object o1,
      final Object o2, final Object unused2) {
    for (final Object item : array) {
      ByteBuffer buffer = null;
      try (final UnusedExceptionHandlers1 u = doSomething2(o1, "", o2)) {
        try (final FileInputStream fis = new FileInputStream(u.getFilename())) {
          final FileChannel fileChannel = fis.getChannel();
          buffer = fileChannel.map(FileChannel.MapMode.READ_ONLY, 0, 42);
        } catch (final IOException e) {
          // ignore
        }
      } catch (final IOException e) {
        // ignore
      }
    }
  }

  private String getFilename() {
    return null;
  }

  private static UnusedExceptionHandlers1 doSomething2(final Object o1, final String s,
      final Object o2) {
    return null;
  }

  @Override
  public void close() throws IOException {}
}

@Away-pp
Copy link
Contributor Author

Away-pp commented Jan 26, 2024

Ok, I will add this test also.
I have compiled an APK with this method, but could not trigger NPE with dx.
It seemed ok.

@skylot
Copy link
Owner

skylot commented Feb 4, 2024

@Away-pp great! Thank you for your work 👍

@skylot skylot merged commit 276ee53 into skylot:master Feb 4, 2024
5 checks passed
nitram84 added a commit to nitram84/jadx that referenced this pull request Feb 15, 2024
skylot added a commit that referenced this pull request Jul 28, 2024
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.

3 participants