java.lang.IllegalAccessError after shrinking code with R8 shrinker wrong method is called [136195382]

Fixed

Bug

NeedsInfo

Status Update

No update yet.

Description

fr...@gmail.com

created issue #1

Jun 28, 2019 05:07PM

Run across this issue after switching to R8 shrinker from proguard.

Android Gradle plugin 3.4

-dontobfuscate enabled for easier debugging
Logcat:
```
java.lang.IllegalAccessError: Method 'java.lang.String com.mapbox.core.MapboxService.baseUrl()' is inaccessible to class 'com.mapbox.api.directions.v5.DirectionsResponseFactory' (declaration of 'com.mapbox.api.directions.v5.DirectionsResponseFactory' appears in /data/app/com.mapbox.services.android.navigation.app-DeTpyD0zikNY9Pe-wE5e5g==/base.apk!classes2.dex)
at com.mapbox.api.directions.v5.DirectionsResponseFactory.generateRouteOptions(DirectionsResponseFactory.java:78)
at com.mapbox.api.directions.v5.DirectionsResponseFactory.generate(DirectionsResponseFactory.java:31)
at com.mapbox.api.directions.v5.MapboxDirections$1.onResponse(MapboxDirections.java:184)
at retrofit2.ExecutorCallAdapterFactory$ExecutorCallbackCall$1$1.run(ExecutorCallAdapterFactory.java:70)
at android.os.Handler.handleCallback(Handler.java:873)
at android.os.Handler.dispatchMessage(Handler.java:99)
at android.os.Looper.loop(Looper.java:193)
at android.app.ActivityThread.main(ActivityThread.java:6863)
at java.lang.reflect.Method.invoke(Native Method)
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:537)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:858)
```
Bytecode with call resulting in crash:
com.mapbox.api.directions.v5.DirectionsResponseFactory
```
.line 78
invoke-virtual {v6}, Lcom/mapbox/core/MapboxService;->baseUrl()Ljava/lang/String;

move-result-object v6

invoke-virtual {v5, v6}, Lcom/mapbox/api/directions/v5/models/RouteOptions$Builder;->baseUrl(Ljava/lang/String;)Lcom/mapbox/api/directions/v5/models/RouteOptions$Builder;

iget-object v6, p0, Lcom/mapbox/api/directions/v5/DirectionsResponseFactory;->mapboxDirections:Lcom/mapbox/api/directions/v5/MapboxDirections;
```
Actual code and referenced class:
com.mapbox.api.directions.v5.MapboxDirections 75:
```
.baseUrl(mapboxDirections.baseUrl())
```
where mapboxDirections is instance of com.mapbox.api.directions.v5.MapboxDirections not MapboxService

Issue was spotted in app i am working on using third party sdk i reproduced it in sample from sdk provider with updated config to run R8 (it requires token to run but analysing APK which can be done without providing it shows the problem) :

https://github.com/freszu/mapbox-navigation-android

Comments

mk...@google.com <mk...@google.com> #2Jun 28, 2019 05:16PM

Assigned to mk...@google.com.

Thank you for the report. There is a high probability that this error has already been fixed. Can I ask you to try the version 1.4.95 by adding the following to your build.gradle

buildscript {

repositories {
maven {
url '

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

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

fr...@gmail.com <fr...@gmail.com> #3Jun 28, 2019 05:37PM

Sadly no success problem is still reproducible.
Tested with 1.4.95 and 1.5.54(Newest version suggested in AS) in both cases i got
Current version is: 1.4.95 (build db845e3ffe68c2d256c91c290c0ad2f0f445c24f from go/r8bot (luci-r8-ci-archive-0-t4qj)).
and
Current version is: 1.5.54 (build 347b4a25c8991312517a2129625fe076bcf0a710 from go/r8bot (luci-r8-ci-archive-0-7c6g)).
in build output so proper version suggested by you and later on by AS was used.

mk...@google.com <mk...@google.com> #4Jun 28, 2019 05:49PM

Thanks for trying those versions out and thank you for supplying the sample. I will take a look.

mk...@google.com <mk...@google.com> #5Jul 1, 2019 10:58AM

I have a small working reproduction of this issue. Basically, we have the following:

package com.mapbox.api.directions.v5;

public abstract class MapboxDirections extends MapboxService<DirectionsResponse, DirectionsService> {
...
protected abstract String baseUrl();
...
}

package com.mapbox.core;

public abstract class MapboxService<T, S> {
...
protected abstract String baseUrl();
...
}

R8 fails to observe that the protected in MapboxDirections.baseUrl() acts like a visiblity bridge. I will start working on a fix.

mk...@google.com <mk...@google.com> #6Jul 1, 2019 11:29AM

For a quick fix you could add the following keep rule:
-keep class com.mapbox.api.directions.v5.MapboxDirections { protected <methods>; }

which will keep all visibility bridges.

mk...@google.com <mk...@google.com> #7Jul 2, 2019 03:32PM

The problem should now be fixed in version d47691811acac217ff214b25d9da6d45b6cda46a. Could you try it out by adding the following to your project build.gradle:

buildscript {

repositories {
maven {
url "

http://storage.googleapis.com/r8-releases/raw/master"
}
}

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

fr...@gmail.com <fr...@gmail.com> #8Jul 2, 2019 03:46PM

This indeed resolves the problem thanks a lot for quick response and action.👏

mk...@google.com <mk...@google.com> #9Jul 3, 2019 09:40AM

Marked as fixed.

Perfect - thank you for trying it out and reporting the error.

ap...@google.com <ap...@google.com> #10Jul 3, 2019 12:07PM

Project: r8
Branch: d8-1.5

commit 93221b04c9c33dc8d7fd19507ddf0286086e6050
Author: Morten Krogh-Jespersen <mkroghj@google.com>
Date: Wed Jul 03 10:22:17 2019

Version 1.5.55

Cherry-pick: Test for incorrect removal of abstract visibility bridge
CL:

https://r8-review.googlesource.com/c/r8/+/40152

Cherry-pick: Consider package name when comparing protected
accessflags
CL:

https://r8-review.googlesource.com/c/r8/+/40154

Bug: 136195382
Change-Id: I1f365864387c3da1ba323c07c4991a5289d66594

M src/main/java/com/android/tools/r8/Version.java
M src/main/java/com/android/tools/r8/graph/AccessFlags.java
M src/main/java/com/android/tools/r8/shaking/ScopedDexMethodSet.java
M src/main/java/com/android/tools/r8/shaking/StaticClassMerger.java
A src/test/java/com/android/tools/r8/shaking/b136195382/AbstractBridgeInheritTest.java
A src/test/java/com/android/tools/r8/shaking/b136195382/package1/Factory.java
A src/test/java/com/android/tools/r8/shaking/b136195382/package1/Service.java
A src/test/java/com/android/tools/r8/shaking/b136195382/package2/Main.java
A src/test/java/com/android/tools/r8/shaking/b136195382/package2/SubFactory.java
A src/test/java/com/android/tools/r8/shaking/b136195382/package2/SubService.java

https://r8-review.googlesource.com/40301

an...@google.com <an...@google.com> #11Jul 3, 2019 03:30PM

The following changes were cherrypicked through Coastguard (

https://android-build.googleplex.com/coastguard/)
Release Track: (

https://android-build.googleplex.com/coastguard/dashboard/5109622965534720/#/cherrypickHistory/all)
Created: 2019-07-03 15:25:45.360785+00:00
Changes: ag/8421710