IssueTracker

Also this should be handled like Activities with results. Some stubbing of intent should be possible and then checking permission result. just like the espresso intents are done.

I've implemented a solution which leverages wrapper classes, overriding and build variant configuration. The solution is quite long to explain and is found over here: https://github.com/ahasbini/AndroidTestMockPermissionUtils.

It is not yet packed in an sdk but the main idea is to override the functionalities of ContextWrapper.checkSelfPermission and ActivityCompat.requestPermissions to be manipulated and return mocked results tricking the app into the different scenarios to be tested like: permission was denied hence the app requested it and ended with granted permission. This scenario will occur even if the app had the permission all along but the idea is that it was tricked by the mocked results from the overriding implementation.

This bug has not been updated in over a year. Please reopen if this is still an issue or requires addition inspection.

I have not explicitly enabled multidex and the buildscript says:
```
Total methods in SD_Maid-v4.14.4(41404)-BETA-9f28fc339.apk: 35080 (53.53% used)
Total fields in SD_Maid-v4.14.4(41404)-BETA-9f28fc339.apk:  26783 (40.87% used)
Methods remaining in SD_Maid-v4.14.4(41404)-BETA-9f28fc339.apk: 30455
Fields remaining in SD_Maid-v4.14.4(41404)-BETA-9f28fc339.apk:  38752
```

So I would assume it's not multidex.

It is not, I validated on the apk

This is not crashing on a 4.4 emulator, flashing a nexus 4 to kit kat to try there

Crashing on a 4.1 (GAPIS) x86 emulator.
Will try 4.2 and 4.3.

The 4.4 crash is from a user device a ALPS GT-I9600@4.4.2 (at least that's what it reports) so that may be another outlier.

AFAIK 4.4 had experimental ART support, maybe that's also a cause for the outlier.

Not crashing on the 4.4 emulator here either, crashing on 4.3 though.

Repro on 4.0 for me, will take a look

I see this right before in the logcat:
W/dalvikvm( 2569): Verifier rejected class Lf/b/a/s/o/i;
W/dalvikvm( 2569): threadid=34: thread exiting with uncaught exception (group=0xb1151228)
I/dalvikvm( 2569): Could not find method android.content.pm.PackageInfo.getLongVersionCode, referenced from method f.b.a.s.a.h.i
W/dalvikvm( 2569): VFY: unable to resolve virtual method 5655: Landroid/content/pm/PackageInfo;.getLongVersionCode ()J

I also see that, but the same thing happens when I comment the related call out.
I only use this method in one place, and there it's guarded:

```
    @Override
    public long getVersionCode() {
        if (ApiHelper.hasAndroidP()) {
            return packageInfo.getLongVersionCode();
        } else {
            //noinspection deprecation
            return packageInfo.versionCode;
        }
    }
    public static boolean hasAndroidP() {
        return Build.VERSION.RELEASE.equals("P") || SDK_INT >= Build.VERSION_CODES.P;
    }
```

Looking at the disassembly at the end of this method could indicate that we are hitting a workaround that we had for other issues:
#
# Method: 'e':
#

void f.b.a.s.o.k.e()
registers: 13, inputs: 1, outputs: 9
------------------------------------------------------------
inst#  offset  instruction         arguments
------------------------------------------------------------
    0:   0x00: MonitorEnter        v12
         0x01, line 1, locals: [12 -> this]
<SNIP>
  110:   0xd8: MonitorExit         v12
  111:   0xd9: Throw               v0
  112:   0xda: ReturnVoid          
Tries (numbers are offsets)
  [0x01 .. 0x21] -> 0
<SNIP>

The ReturnVoid at the end here is not reachable (it is a workaround for a different bug), but since we had other issues with this, can I convince you to try a new version of R8 (if there is a studio 3.4.1 release this will be in there). This version will not produce unreachable code in this case.

You can test this out by changing your gradle file to:
buildscript {

    repositories {
        maven {
            url 'http://storage.googleapis.com/r8-releases/raw'
        }
    }

    dependencies {
        classpath 'com.android.tools:r8:1.4.87'  // Must be before the Gradle Plugin for Android.
        classpath 'com.android.tools.build:gradle:YOUR VERSION'
     }
}

re #13:
Yes, that is not the problem I think

Hm, I added it to my project level build.gradle?


```
buildscript {
    ext.kotlin_version = '1.3.30'
    repositories {
        maven {
            url "http://storage.googleapis.com/r8-releases/raw/master"
        }
        jcenter()
        google()
        mavenCentral()
    }
    dependencies {
        classpath 'com.android.tools:r8:1.4.87'
        classpath 'com.android.tools.build:gradle:3.4.0'
        classpath 'com.getkeepsafe.dexcount:dexcount-gradle-plugin:0.6.4'
        classpath 'com.bugsnag:bugsnag-android-gradle-plugin:3.6.0'
        classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlin_version"
    }
}
```

```
Could not find com.android.tools:r8:1.4.87.
Searched in the following locations:
  - file:/snap/android-studio/75/android-studio/gradle/m2repository/com/android/tools/r8/1.4.87/r8-1.4.87.pom
  - file:/snap/android-studio/75/android-studio/gradle/m2repository/com/android/tools/r8/1.4.87/r8-1.4.87.jar
  - http://storage.googleapis.com/r8-releases/raw/master/com/android/tools/r8/1.4.87/r8-1.4.87.pom
  - http://storage.googleapis.com/r8-releases/raw/master/com/android/tools/r8/1.4.87/r8-1.4.87.jar
  - https://jcenter.bintray.com/com/android/tools/r8/1.4.87/r8-1.4.87.pom
  - https://jcenter.bintray.com/com/android/tools/r8/1.4.87/r8-1.4.87.jar
  - https://dl.google.com/dl/android/maven2/com/android/tools/r8/1.4.87/r8-1.4.87.pom
  - https://dl.google.com/dl/android/maven2/com/android/tools/r8/1.4.87/r8-1.4.87.jar
  - https://repo.maven.apache.org/maven2/com/android/tools/r8/1.4.87/r8-1.4.87.pom
  - https://repo.maven.apache.org/maven2/com/android/tools/r8/1.4.87/r8-1.4.87.jar
Required by:
    project :
```

Try with the following repository instead:

        maven {
            url 'http://storage.googleapis.com/r8-releases/raw'
        }

Re #17, Success!

Re #14, No Success :( Same error.

can you share the apk again?

Here:
https://www.dropbox.com/s/adig65r6d310ayp/SD_Maid-v4.14.4%2841404%29-BETA-9f28fc339.apk?dl=0

Poking a bit more around.
I narrowed it down a bit more to a class I can share.

It seems to be related to my class `StorageVolumeMapper`.

StorageManager.load() initializes StorageFactory(..., volumeMapperLazy, ...)

volumeMapperLazy is a dagger.Lazy<StorageVolumeMapper>

If I comment out the below code then the crash doesn't happen, but due to the API guard, it should effectively be commented out on < Android 5.1 anyways?

```
if (ApiHelper.hasLolliPop()) {
    StorageVolumeMapper mapper = storageVolumeMapperLazy.get();
    for (Collection<Storage> storages : locationListMap.values()) {
        for (Storage storage : storages) {
            storage.setLabel(mapper.getLabelForStorage(storage));
        }
    }
}
```

More specifically, if I comment out StorageVolumeMapper.java#L141 the crash no longer happens.

Which is weird because init() gets called in the constructor, but doesn't cause a crash.
And init() returns immediately anways due to
```
if (!ApiHelper.hasLolliPop()) return false;
```

Here is the file:
https://gist.github.com/d4rken/3df8797188080fa5a8056954d62369bb

Adding @Keep basically anywhere within StorageVolumeMapper.java also fixes this.

Can you share an apk where this is fixed?

See attachment.

To keep the bug updated.
The hypothesis was that a move before init call was triggering this:
         0x64, line 14, locals: [12 -> this]
   50:   0x64: NewInstance         v0, f.b.a.s.o.i
   51:   0x66: IgetObject          v4, v12, Field android.content.Context f.b.a.s.o.k.f
   52:   0x68: IgetObject          v5, v12, Field f.b.a.qa f.b.a.s.o.k.i
   53:   0x6a: IgetObject          v6, v12, Field f.b.a.s.A f.b.a.s.o.k.g
   54:   0x6c: IgetObject          v7, v12, Field d.a f.b.a.s.o.k.h
   55:   0x6e: IgetObject          v9, v12, Field f.b.a.s.f.a f.b.a.s.o.k.j
   56:   0x70: IgetObject          v11, v12, Field java.util.ArrayList f.b.a.s.o.k.c
   57:   0x72: MoveObject          v3, v0  <---- HERE
   58:   0x73: MoveObject          v8, v2
   59:   0x74: MoveObject          v10, v12
   60:   0x75: InvokeDirectRange   { v3 .. v11 } Lf/b/a/s/o/i;-><init>(Landroid/content/Context;Lf/b/a/qa;Lf/b/a/s/A;Ld/a;Lf/b/a/s/n/b;Lf/b/a/s/f/a;Lf/b/a/s/o/k;Ljava/util/Collection;)V
If the verifier somehow thought that this was a move of an object where we had not yet called init, that could trigger this (although we should probably have seen that before then, this pattern should be relatively common)
The apk from #24 shows that this is not the problem, the whole f.b.a.s.o.k.e method is identical between the two.

I can baksmali and smali back to dex, and repro the issue, this should make it easier to figure out

OK, so I finally figured this out:
The actual error was further up in the logcat:
W/dalvikvm(31115): VFY: unable to resolve exception class 6812 (Ljava/lang/ReflectiveOperationException;)
W/dalvikvm(31115): VFY: unable to find exception handler at addr 0x1ba
W/dalvikvm(31115): VFY:  rejected Lf/b/a/s/o/i;.g ()Ljava/util/Map;
W/dalvikvm(31115): VFY:  rejecting opcode 0x0d at 0x01ba
W/dalvikvm(31115): VFY:  rejected Lf/b/a/s/o/i;.g ()Ljava/util/Map;
W/dalvikvm(31115): Verifier rejected class Lf/b/a/s/o/i;
W/dalvikvm(31115): threadid=36: thread exiting with uncaught exception (group=0xb1151228)

So basically, we are rejecting the method because we are trying to catch an exception that only started existing on API level 19:
https://developer.android.com/reference/java/lang/ReflectiveOperationException

Catch handlers:
Tries (numbers are offsets)
  [0x18a .. 0x193] -> 0
  [0x195 .. 0x1b6] -> 1
  [0x1cf .. 0x20c] -> 2
  [0x21d .. 0x24c] -> 3
  [0x24f .. 0x257] -> 4
  [0x25e .. 0x266] -> 5
  [0x268 .. 0x270] -> 6
  [0x277 .. 0x27f] -> 7
Handlers (numbers are offsets)
  0: [
       java.lang.ReflectiveOperationException -> 0x1bc
     ]
  1: [
       java.lang.ReflectiveOperationException -> 0x1ba
     ]
<SNIP>

The place where this goes wrong in StorageFactory.java, although this might be inlined from somewhere else. Is it possible to share StorageFactory class file and your mapping file (if not publicly, then with ricow@google.com)

Not publicly, I've send you an invite to a GDrive folder.

This is reproducible by simply doing this in an apk with
minSdkVersion 16
targetSdkVersion 28

Then adding a method like this:
  public void foobar() {
        if (Build.VERSION.SDK_INT >= 19) {
            try {
                throw new ReflectiveOperationException("foobar");
            } catch (ReflectiveOperationException e) {
                System.out.println(e.getMessage());
            }
        }
    }
Because the type is used in a catch handler, it does not matter if it is guarded by the version, the verifier will not complete.

And this is reproducible using d8 and not calling the foobar method, this is simply in the class verification

This also explains the strangeness in #21, when you comment out the init call in StorageVolumeMapper.java#L141 we probably no long inline it. Looking at the map file you provided by mail:
    44:48:java.util.Map build():185:189 -> g
    49:49:void eu.thedarken.sdm.tools.storage.Storage.setLabel(java.lang.String):137:137 -> g
    49:49:java.util.Map build():189 -> g
That seems to indicate that it is the working version.

When this is failing, I bet we are a method from StorageVolumeMapper when doing storage.setLabel(mapper.getLabelForStorage(storage));
When that happens, the code that has the triggering catch handler is in class, and is verified (even when it is not reachable due to version checks)
Before, StorageVolumeMapper would never be instantiated from your code, so the verifier would not run on that code.

Also, the reason that the keep rules work is that when the code is keept in that class, we don't inline it (since we normally only inline if code has a single call site or is very small).

That said, this is clearly a verifier bug, but there is nothing we can do about it on these old phones. Using the ReflectiveOperationException in a method, e.g., throwing it, as long as that code is not reachable, works fine - this only happens when it is defining a catch block.
I am unsure if we can do anything in studio to let people know.

I really tend to always find those obscure bugs ¯\_(ツ)_/¯

So the R8 inlining causes this, and this inlining is something new in R8 (more optimizations)?

But this isn't a bug in R8, it only reveals a bug in the verifier, which is that verification doesn't understand that that code (or rather that catch) is unreachable due to the SDK_INT check?

This doesn't happen on newer versions because ReflectiveOperationException is known, but does the verifier bug itself still exists up until the current Android version?

Maybe CC some Android-Studio LINT folks? I think this should be possible to flag via some LINT rule magic?

You are correct in your assessment.
Yes, I will talk with the LINT people.
I doubt that this is still an issue in ART, but +ngeoffray to verify

Would you concur that the @Keep is best workaround for this?

Keeps the workaround in the same file (not in some proguard file) and the only potential sideeffect is one (not so large class) not being optimized (as well as possible?), or does the @Keep break other optimizations besides inlining?

I agree that @Keep is your best bet, alternatively, if you can catch the more specific exceptions(s) (ClassNotFoundException, IllegalAccessException, InstantiationException, InvocationTargetException, NoSuchFieldException, NoSuchMethodException) then you will not have this problem (since they are available from api level 1)

+tnorbye@
This does not seem to be widespread, but we could provide warnings for this.
TL;DR: If a class has a method that catches ReflectiveOperationException or another Exception that is not available on pre 19 (no matter if we call that method  or not, as long as we instantiate the class), the dalvik verifier will crash the app
Studio correctly tells me that the class is not available on pre-19, but if I put it into a conditional, there is no red flags:
if (Build.VERSION.SDK_INT >= 19) {
    try {
        throw new ClassNotFoundException("foobar");
    } catch (ReflectiveOperationException e) {
        System.out.println(e.getMessage());
    }
}
(but this still crashes dalvik on 4.04 - 4.3)

Also, I validated that starting from api level 21 we don't see this, I made a smali reproduction, where I simply caught a non existing exception. This crashes in the same way all the way up to 4.4.4(dalvik), but not on 5.0 (art)

Interesting -- so just *loading* a class that references ReflectiveOperationException can crash if the app minSdkVersion <= 19?

#38: in this case, it's when the reference is from a catch clause.
#33: I can confirm this is fixed in ART.

Thanks for confirming Nicolas, that also matches my small experiment in #37.
Tor: As Nicolas says, this is only in a catch class, as the actual exception being caught.

If we load a class, the verifier will run on it, first method below is fine, we can have that in any class as long as we don't call it on <= 19.

public void no_problem_unless_called() {
  throw new ReflectiveOperationException("foobar");
}

This method will crash dalvik if it is in a class we load, no matter if we call it or not.

public void crash_even_if_never_called() {
  try {
    no_problem_unless_called();
  } catch (ReflectiveOperationException e) { }
}

It is fine to have this method in a class that is not loaded (which is what the reporter of this bug used to have, but now R8 inlined code, including the try-catch with ReflectiveOperationException into a class that was loaded (this other class we inline from is never loaded on <=19). The code in that block is under a "if (Build.VERSION.SDK_INT >= 19)" equivalent, but that does not help, the verifier still crashes.

This is not exclusively ReflectiveOperationException, any non existing exception in a catch clause will do. Any vm (at least down to 4.0.4) before L will crash on the dex for the following smali (but art correctly handles it):
.method public static foobar()V
    .registers 2

    .line 8
    sget v0, Landroid/os/Build$VERSION;->SDK_INT:I

    const/16 v1, 0x1b

    if-lt v0, v1, :cond_12 <------- HERE: Only do this on API <27

    .line 10
    :try_start_6
    invoke-static {}, LTest$Foobar;->something()V
    :try_end_9
    .catch Ljava/lang/X; {:try_start_6 .. :try_end_9} :catch_a  <---------------- HERE: none existing exception in catch clause

    .line 13
    goto :goto_12

    .line 11
    :catch_a
    move-exception v0

    .line 12
    sget-object v0, Ljava/lang/System;->out:Ljava/io/PrintStream;

    const-string v1, "foobar"

    invoke-virtual {v0, v1}, Ljava/io/PrintStream;->println(Ljava/lang/String;)V

    .line 15
    :cond_12
    :goto_12
    return-void
.end method

Rico will rewriting the ode to something like this work on these VMs?

  ...
  if (Build.VERSION.SDK_INT >= 19) {
    x();
  }
  ...

void x() {
  try {
    throw new ClassNotFoundException("foobar");
  } catch (ReflectiveOperationException e) {
    System.out.println(e.getMessage());
  }
}

#41 as discussed offline, not if x is in the same class (and there are other issues in moving it out)
Offline discussion:
1. We could try to fix this in R8, by rewriting the catch to instead catch Exception, then rethrow if it is not ReflectiveOperationException, with all remaining catch handlers having their range include the rethrow instruction. -
 - This is complicated to do
2. We could not inline methods that have a ReflectiveOperationException catch handler if api level is <19
 - This is relatively easy to do, and would have prevented the problem here, but people can still easily do code like #36 that makes them crash if they don't have the catch ReflectiveOperationException in a class that never instantiated on <19.
 - On fix for that is to warn about it in studio, and tell people to not catch ReflectiveOperationException on <19. If a library throws the ReflectiveOperationException (i.e., not one of the subclasses, but the actual class) then you are out of luck.

Landed "fix" for not inlining method with ReflectiveOperationException in catch handlers (as well as not inlining unknown classes in catch handlers) in:
https://r8.googlesource.com/r8/+/67c432190a6598ea0c89794c1758f4dde962bb09

Filed studio lint tracking bug for warning on usage if target is pre 19:
https://b.corp.google.com/issues/131860415

Closing this as fixed on our side

Thanks for all the details, very interesting.

Which R8 version will include this?

This will be in the a canary version in the not to distant future, and hit 3.5 beta2 or beta3
You should be able to try it out with:
    repositories {
        maven {
            url "http://storage.googleapis.com/r8-releases/raw/master"
        }
    }

    dependencies {
        classpath 'com.android.tools:r8:67c432190a6598ea0c89794c1758f4dde962bb09'  // Must be before the Gradle Plugin for Android.
        classpath 'com.android.tools.build:gradle:X.Y.Z'     // Your current AGP version.
     }

Hi Rico, can you confirm that you went with fix #2 from comment #42? In other words, even if one is using a version of R8 with the fix, the example in #36 will crash if that class is ever loaded?  What I'm planning on doing in lint is to make the code which *normally* checks to see if an API reference is surrounded by a version check, for catch clause class references specifically instead *only* see if the top level class itself is annotated with @RequiresApi (and update the error message to point out that any other surrounding version checks in the class does not handle this scenario). That way, you're clued in to the fact that any *references* from other places to this class must be surrounded by a version check. If the code in #36 was in a class that is referenced nowhere in the app, it wouldn't crash right? And if it was referenced from within a version check in another class, the app also wouldn't crash right? It's only if the class itself is loaded, which would happen if a statement is encountered on that device which references the type?

#42: Yes I can confirm we went with the option #2.
You are correct, this is only if this is in a class that is loaded. This bug report is an example of this, the class including the catch clause with ReflectiveOperationException was in a class that was never loaded on <19.
I like the idea of forcing people to annotate all classes that have these exceptions in catch clauses, that way they should not encounter this.

FYI lint/Studio will now in 3.6 (and in 3.5 as soon as ag/7537278 is merged) warn about this situation; this work was tagracked in b/131843501.

s/tagracked/tracked/

#49, thanks, I closed b/131860415 as a duplicate of b/131843501