square / okio

A modern I/O library for Android, Java, and Kotlin Multiplatform.
https://square.github.io/okio/
Apache License 2.0
8.79k stars 1.18k forks source link

ArrayIndexOutOfBoundsException at okio.Base64.encode(Base64.java:137) #325

Closed guillaumewrobel closed 6 years ago

guillaumewrobel commented 6 years ago

OS:

Android 5.1 100%

Devices:

Archos 36% condor 27% Meizu 23% HUAWEI 14%

Stacktrace:

Fatal Exception: java.lang.ArrayIndexOutOfBoundsException: length=1840; index=320371712
       at okio.Base64.encode(Base64.java:137)
       at okio.Base64.encode(Base64.java:125)
       at okio.ByteString.base64(ByteString.java:131)
       at okhttp3.Cache$Entry.writeCertList(Cache.java:683)
       at okhttp3.Cache$Entry.writeTo(Cache.java:646)
       at okhttp3.Cache.put(Cache.java:250)
       at okhttp3.Cache$1.put(Cache.java:149)
       at okhttp3.internal.cache.CacheInterceptor.intercept(CacheInterceptor.java:131)
       at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
       at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:121)
       at okhttp3.internal.http.BridgeInterceptor.intercept(BridgeInterceptor.java:93)
       at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
       at okhttp3.internal.http.RetryAndFollowUpInterceptor.intercept(RetryAndFollowUpInterceptor.java:125)
       at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
       at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:121)
       at okhttp3.logging.HttpLoggingInterceptor.intercept(HttpLoggingInterceptor.java:143)
       at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:147)
       at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.java:121)
       at okhttp3.RealCall.getResponseWithInterceptorChain(RealCall.java:200)
       at okhttp3.RealCall$AsyncCall.execute(RealCall.java:147)
       at okhttp3.internal.NamedRunnable.run(NamedRunnable.java:32)
       at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1112)
       at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:587)
       at java.lang.Thread.run(Thread.java:818)

Many Picasso-related tasks in the other threads.

Can provide further info if needed

mtrakal commented 6 years ago

Android: 5.1 - 100%

Fabric crashes, more than 500 affected users per day in last 7 days

Devices:

Meizu27%
LENOVO25%
HUAWEI17%
asus9%
Other…22%
Fatal Exception: java.lang.ArrayIndexOutOfBoundsException: length=2308; index=865832320
       at okio.Base64.encode(SourceFile:137)
       at okio.Base64.encode(SourceFile:125)
       at okio.ByteString.base64(SourceFile:131)
       at okhttp3.Cache$Entry.writeCertList(SourceFile:683)
       at okhttp3.Cache$Entry.writeTo(SourceFile:646)
       at okhttp3.Cache.put(SourceFile:250)
       at okhttp3.Cache$1.put(SourceFile:149)
biokys commented 6 years ago

We see the same issue from 8 of November. Affecting many libraries using okhttp -> okio.

swankjesse commented 6 years ago

So there's a VM bug on many Chinese devices running Android 5.1. To fix this we need to figure out what the VM bug is and how to work around it. Does anyone have a device that can reproduce this consistently?

mtrakal commented 6 years ago

We are going to buy one, so after wi will try to reproduce it and try to check what's happen. On AVD, Genymotion and Samsung virtual devices we can't reproduce it. We will try to provide more info later

ghost commented 6 years ago

I have the same issue. This devices crash constantly : Flare S4 lite HUAWEI Y6 Pro

mtrakal commented 6 years ago

We bought Meizu M2, Andorid OS: 5.1, Flyme OS: 4.5.30, I could install app without any crash :/. But a lot of our users can't run app on same device and I think, same os version. So sorry, I still can't reproduce it :/. I'll still try to reproduce this issue

DavidEdwards commented 6 years ago

My users have the same issue. Crashlytics receives about 20 of these crashes a day.

I am unable to reproduce the issue.

Acer - Iconia One 10 and TCL appear to be major culprits.

image

image

Incidentally, in case it is relevant, I have been seeing this error since 1st November.

mtrakal commented 6 years ago

Can you @DavidEdwards, @umbrella-alexander-ageychenko and @GuillaumeWrobel provide your app google play links? I'll try to install on our device to check it. Do you know when your apps crash? Our app https://play.google.com/store/apps/details?id=com.droid4you.application.wallet crashed while login, but not for me :/

pavlus commented 6 years ago

@mtrakal can you try with Flyme 6.1.0.0G? We had reports from m2 with this OS version.

donatienthorez commented 6 years ago

Same for us but for "LAVA" phones and some others :

screen shot 2017-11-14 at 15 54 36 screen shot 2017-11-14 at 15 54 25

Here is the link for the play store : https://play.google.com/store/apps/details?id=com.cashquizz.cashquizz. From the log, I guess that the app crashes when it launches.

mtrakal commented 6 years ago

@donatienthorez app on Meizu M2, Andorid OS: 5.1, Flyme OS: 4.5.30 works withotu issue, I can log-in...

@pavlus I'm going to try Flyme 6.1.0.0G, we'll see

mtrakal commented 6 years ago

@donatienthorez on Flyme 6.1.0.0G I can reproduce crash on ArrayIndexOutOfBoundsException in your app and in our app to!

DavidEdwards commented 6 years ago

I cannot provide a link to my App, but it seems like you don't need it now anyway.

What I do notice, is that these crashes appeared abruptly, coinciding with the update of my App. Very little was changed, but notably some of the underlying dependencies had their versions updated. This could be entirely coincidental.

-    buildToolsVersion "26.0.1"
+    buildToolsVersion "27.0.0"
-    classpath 'com.android.tools.build:gradle:3.0.0-beta5'
+    classpath 'com.android.tools.build:gradle:3.1.0-alpha01'
-    compile "com.android.support:multidex:1.0.1"
-    compile "com.android.support:support-annotations:26.0.1"
-    compile "com.android.support:support-v4:26.0.1"
-    compile "com.android.support:gridlayout-v7:26.0.1"
-    compile "com.android.support:appcompat-v7:26.0.1"
-    compile "com.android.support:mediarouter-v7:26.0.1"
-    compile "com.android.support:design:26.0.1"
-    compile "com.android.support:support-v13:26.0.1"
-    compile "com.android.support:palette-v7:26.0.1"
-    compile "com.android.support:cardview-v7:26.0.1"
+    compile "com.android.support:multidex:1.0.2"
+    compile "com.android.support:support-annotations:26.0.2"
+    compile "com.android.support:support-v4:26.0.2"
+    compile "com.android.support:gridlayout-v7:26.0.2"
+    compile "com.android.support:appcompat-v7:26.0.2"
+    compile "com.android.support:mediarouter-v7:26.0.2"
+    compile "com.android.support:design:26.0.2"
+    compile "com.android.support:support-v13:26.0.2"
+    compile "com.android.support:palette-v7:26.0.2"
+    compile "com.android.support:cardview-v7:26.0.2"
donatienthorez commented 6 years ago

Just in case it can help, I just checked and from the version we have the bug we also made some updates. buildToolsVersion from 25.0.2 to 26.0.2 gradle from 3.0.0-beta4 to 3.1.0-alpha01.

mtrakal commented 6 years ago

We use different versions of buildTools, multidex we have for longer time on 1.0.2, and support libs we have 27.0.0.

So only one thing which is almost same is com.android.tools.build:gradle:3.1.0-alphaXX We upgraded from 01 to 02 at 8th, but 8th our app start crashing, but before we didn't update for longer time, so 01 we maybe skiped.

Can someone else confirm, that use alpha channel of com.android.tools.build:gradle:3.1.0-alphaXX too?

Interesting is that I can run app in debug flavor (without proguard)

DavidEdwards commented 6 years ago

@GuillaumeWrobel @biokys @pavlus @umbrella-alexander-ageychenko Did any of you update your gradle plugin to com.android.tools.build:gradle:3.1.0-alphaXX?

@mtrakal My Android Studio is reporting a new version: 3.1.0-alpha03. Does that have the same issue for you?

mtrakal commented 6 years ago

3.1.0-alpha03 have same issue, now I try to build version with stable 3.0.0

Edit: reverting to 3.0.0 doesn't solve anything, so maybe it's something else :/

DavidEdwards commented 6 years ago

Alright. It was a long shot to begin with. Thank you for testing it. You could try reverting to a commit a month or two ago in your project. If that is possible.

mtrakal commented 6 years ago

it has something to do with proguard, when proguard is disabled this error doesn't occur... But I still don't have working solution

mtrakal commented 6 years ago

okhttp3 > Credentials.basic(String userName, String password, Charset charset) { return on line 35 String encoded = ByteString.encodeString(usernameAndPassword, charset).base64(); new line in ByteString.encodeString(usernameAndPassword, charset). So there is error, because base64() can't handle new line.

https://github.com/square/okio/blob/master/okio/src/main/java/okio/ByteString.java#L109

But it's WTF, because I have no idea why it happen :/

s.getBytes(charset) from line L109 come from Android API, so it looks on some issue in this method...

mtrakal commented 6 years ago

I reseted our branch to some 2 month old commit, build apk and install, crashed! I tried to change date in phone, crashed! I tried to factory reset phone (without google play services), install 2 month old apk over adb, crashed!

Fuck it, I lost all my ideas what could it caused :/

It has something to do with developer tools (studio, gradle, ...), because when I install old apk (builded when it was released) I can install and run without crash :). So, maybe android studio, or some another building tools, but I build this builds over console, so Studio should be excluded... Maybe JDK or Android SDK...

pavlus commented 6 years ago

@mtrakal you said it happens when using ProGuard, can you play around with it's config, like -dontoptimize on okio package?

pavlus commented 6 years ago

Just tried disassembling okio.Base64 and okio.ByteString from apk with proguard optimizations enabled/disabled, no difference in smali code inside methods.

pavlus commented 6 years ago

May it be, that this specific version of Android changed output of java.security.cert.Certificate.getEncoded()?

Bytes to be converted are coming from here: https://github.com/square/okhttp/blob/master/okhttp/src/main/java/okhttp3/Cache.java#L682

pavlus commented 6 years ago

But looking at message java.lang.ArrayIndexOutOfBoundsException: length=1840; index=320371712 we see that app tries to index byte assuming array contains at least 305 MB of data, that doesn't look right.

Probably some ART optimization broken?

pavlus commented 6 years ago

Fun fact: in all received reports index from the exception message has at least 5 least significant bits and 3 most significant bits set to 0.

Right now I'm assuming that dex2oat messes with bit manipulation instructions.

mtrakal commented 6 years ago

It doesn't depend on Gradle (tested with 4.2.1 - 4.3.1), build toos (26.0.2, 27.0.1), app compat (different versions), kotlin different versions, JDK (1.8.0_144, 151 and JetBrains 152), gradle build tools 3.0.0-3.1.0-alpha03, ... no changes.

Most interesting things is: I disable proguard for our release buildTypes, everything same, only one thing is changed debuggable true and debuggable false. When debuggable true set, app working without any issue (so I can't debug it, because it doesn't crash). When I set debuggable false app start crashing.

cketti commented 6 years ago

Debug builds might disable AOT compilation for faster app installation times during development. Check if vmSafeMode is set to true in the generated manifest.

From what has been reported so far I believe it's likely that dex2oat is either producing incorrect code or is producing code that triggers a bug in the CPU.

The next step should probably be to generate a minimal working example that triggers the bug. Then look at the dex2oat output.

pavlus commented 6 years ago

@cketti ok then, I'm going to implement test app that generates an array of 256*256 bytes (with all possible pairs) and tries to Base64 it with okio and alternative implementations.

mtrakal commented 6 years ago

ok, guys, AND SOLUTION IS...

wait for it....

... just disable D8...

gradle.properties > android.enableD8=false.

Now let's go to pub and drink a lot of beers, because I spend on this shit all 2 DAYS :/.

Tested for you on our big project and on some small demo project just with one library (okhttp3).

JakeWharton commented 6 years ago

That is not a solution. D8 is becoming the default and the underlying cause must be found.

On Wed, Nov 15, 2017, 12:15 PM Matěj Trakal notifications@github.com wrote:

ok, guys, AND SOLUTION IS...

wait for it....

... just disable D8...

gradle.properties > android.enableD8=false.

Now let's go to drink a lot of beers, because I spend on this shit all 2 DAYS :/.

Tested for you on our big project and on some small demo project just with one library (okhtt3).

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/square/okio/issues/325#issuecomment-344662797, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEEEUMvd9iXDkh2pItNv6v7yEpe0gazks5s2xw2gaJpZM4QbaVP .

pavlus commented 6 years ago

Just in case someone want to play around this issue: https://github.com/pavlus/Okio325Test

mtrakal commented 6 years ago

I agree, but it can found someone else.

Someone who is paying for searching this crazy bugs and have higher skill than me :). I'm just dummy programmer copypaster from stackoverflow...

I reporting every week some issue in AS or Android to Google, but I have no more energy for it. Going to beer. Sorry, but this issue took me complete 2 working days... Now is night in my country.

mtrakal commented 6 years ago

https://github.com/mtrakal/okio325 demo project for reproducing this issue

cketti commented 6 years ago
Dalvik bytecode for Base64.encode(byte[], byte[]) compiled with D8 ``` .method private static encode([B[B)Ljava/lang/String; .registers 10 .line 133 array-length v0, p0 add-int/lit8 v0, v0, 0x2 div-int/lit8 v0, v0, 0x3 mul-int/lit8 v0, v0, 0x4 .line 134 new-array v0, v0, [B .line 135 array-length v1, p0 array-length v2, p0 rem-int/lit8 v2, v2, 0x3 sub-int/2addr v1, v2 const/4 v2, 0x0 move v3, v2 :goto_10 if-ge v2, v1, :cond_55 .line 137 add-int/lit8 v4, v3, 0x1 aget-byte v5, p0, v2 and-int/lit16 v5, v5, 0xff shr-int/lit8 v5, v5, 0x2 aget-byte v5, p1, v5 aput-byte v5, v0, v3 .line 138 add-int/lit8 v3, v4, 0x1 aget-byte v5, p0, v2 and-int/lit8 v5, v5, 0x3 shl-int/lit8 v5, v5, 0x4 add-int/lit8 v6, v2, 0x1 aget-byte v7, p0, v6 and-int/lit16 v7, v7, 0xff shr-int/lit8 v7, v7, 0x4 or-int/2addr v5, v7 aget-byte v5, p1, v5 aput-byte v5, v0, v4 .line 139 add-int/lit8 v4, v3, 0x1 aget-byte v5, p0, v6 and-int/lit8 v5, v5, 0xf shl-int/lit8 v5, v5, 0x2 add-int/lit8 v6, v2, 0x2 aget-byte v7, p0, v6 and-int/lit16 v7, v7, 0xff shr-int/lit8 v7, v7, 0x6 or-int/2addr v5, v7 aget-byte v5, p1, v5 aput-byte v5, v0, v3 .line 140 add-int/lit8 v3, v4, 0x1 aget-byte v5, p0, v6 and-int/lit8 v5, v5, 0x3f aget-byte v5, p1, v5 aput-byte v5, v0, v4 add-int/lit8 v2, v2, 0x3 goto :goto_10 .line 142 :cond_55 array-length v2, p0 rem-int/lit8 v2, v2, 0x3 const/16 v4, 0x3d packed-switch v2, :pswitch_data_bc goto :goto_ac .line 150 :pswitch_5e add-int/lit8 v2, v3, 0x1 aget-byte v5, p0, v1 and-int/lit16 v5, v5, 0xff shr-int/lit8 v5, v5, 0x2 aget-byte v5, p1, v5 aput-byte v5, v0, v3 .line 151 add-int/lit8 v3, v2, 0x1 aget-byte v5, p0, v1 and-int/lit8 v5, v5, 0x3 shl-int/lit8 v5, v5, 0x4 add-int/lit8 v1, v1, 0x1 aget-byte v6, p0, v1 and-int/lit16 v6, v6, 0xff shr-int/lit8 v6, v6, 0x4 or-int/2addr v5, v6 aget-byte v5, p1, v5 aput-byte v5, v0, v2 .line 152 add-int/lit8 v2, v3, 0x1 aget-byte p0, p0, v1 and-int/lit8 p0, p0, 0xf shl-int/lit8 p0, p0, 0x2 aget-byte p0, p1, p0 aput-byte p0, v0, v3 .line 153 aput-byte v4, v0, v2 goto :goto_ac .line 144 :pswitch_8e add-int/lit8 v2, v3, 0x1 aget-byte v5, p0, v1 and-int/lit16 v5, v5, 0xff shr-int/lit8 v5, v5, 0x2 aget-byte v5, p1, v5 aput-byte v5, v0, v3 .line 145 add-int/lit8 v3, v2, 0x1 aget-byte p0, p0, v1 and-int/lit8 p0, p0, 0x3 shl-int/lit8 p0, p0, 0x4 aget-byte p0, p1, p0 aput-byte p0, v0, v2 .line 146 add-int/lit8 p0, v3, 0x1 aput-byte v4, v0, v3 .line 147 aput-byte v4, v0, p0 .line 157 :goto_ac :try_start_ac new-instance p0, Ljava/lang/String; const-string p1, "US-ASCII" invoke-direct {p0, v0, p1}, Ljava/lang/String;->([BLjava/lang/String;)V return-object p0 .line 158 :catch_b4 move-exception p0 :try_end_b5 .catch Ljava/io/UnsupportedEncodingException; {:try_start_ac .. :try_end_b5} :catch_b4 .line 159 new-instance p1, Ljava/lang/AssertionError; invoke-direct {p1, p0}, Ljava/lang/AssertionError;->(Ljava/lang/Object;)V throw p1 nop :pswitch_data_bc .packed-switch 0x1 :pswitch_8e :pswitch_5e .end packed-switch .end method ```

Dalvik bytecode for Base64.encode(byte[], byte[]) compiled with dx ``` .method private static encode([B[B)Ljava/lang/String; .registers 12 .param p0, "in" # [B .param p1, "map" # [B .prologue const/16 v9, 0x3d .line 133 array-length v7, p0 add-int/lit8 v7, v7, 0x2 div-int/lit8 v7, v7, 0x3 mul-int/lit8 v5, v7, 0x4 .line 134 .local v5, "length":I new-array v6, v5, [B .line 135 .local v6, "out":[B const/4 v3, 0x0 .local v3, "index":I array-length v7, p0 array-length v8, p0 rem-int/lit8 v8, v8, 0x3 sub-int v1, v7, v8 .line 136 .local v1, "end":I const/4 v2, 0x0 .local v2, "i":I move v4, v3 .end local v3 # "index":I .local v4, "index":I :goto_14 if-ge v2, v1, :cond_5d .line 137 add-int/lit8 v3, v4, 0x1 .end local v4 # "index":I .restart local v3 # "index":I aget-byte v7, p0, v2 and-int/lit16 v7, v7, 0xff shr-int/lit8 v7, v7, 0x2 aget-byte v7, p1, v7 aput-byte v7, v6, v4 .line 138 add-int/lit8 v4, v3, 0x1 .end local v3 # "index":I .restart local v4 # "index":I aget-byte v7, p0, v2 and-int/lit8 v7, v7, 0x3 shl-int/lit8 v7, v7, 0x4 add-int/lit8 v8, v2, 0x1 aget-byte v8, p0, v8 and-int/lit16 v8, v8, 0xff shr-int/lit8 v8, v8, 0x4 or-int/2addr v7, v8 aget-byte v7, p1, v7 aput-byte v7, v6, v3 .line 139 add-int/lit8 v3, v4, 0x1 .end local v4 # "index":I .restart local v3 # "index":I add-int/lit8 v7, v2, 0x1 aget-byte v7, p0, v7 and-int/lit8 v7, v7, 0xf shl-int/lit8 v7, v7, 0x2 add-int/lit8 v8, v2, 0x2 aget-byte v8, p0, v8 and-int/lit16 v8, v8, 0xff shr-int/lit8 v8, v8, 0x6 or-int/2addr v7, v8 aget-byte v7, p1, v7 aput-byte v7, v6, v4 .line 140 add-int/lit8 v4, v3, 0x1 .end local v3 # "index":I .restart local v4 # "index":I add-int/lit8 v7, v2, 0x2 aget-byte v7, p0, v7 and-int/lit8 v7, v7, 0x3f aget-byte v7, p1, v7 aput-byte v7, v6, v3 .line 136 add-int/lit8 v2, v2, 0x3 goto :goto_14 .line 142 :cond_5d array-length v7, p0 rem-int/lit8 v7, v7, 0x3 packed-switch v7, :pswitch_data_ca :goto_63 move v3, v4 .line 157 .end local v4 # "index":I .restart local v3 # "index":I :goto_64 :try_start_64 new-instance v7, Ljava/lang/String; const-string v8, "US-ASCII" invoke-direct {v7, v6, v8}, Ljava/lang/String;->([BLjava/lang/String;)V :try_end_6b .catch Ljava/io/UnsupportedEncodingException; {:try_start_64 .. :try_end_6b} :catch_c2 return-object v7 .line 144 .end local v3 # "index":I .restart local v4 # "index":I :pswitch_6c add-int/lit8 v3, v4, 0x1 .end local v4 # "index":I .restart local v3 # "index":I aget-byte v7, p0, v1 and-int/lit16 v7, v7, 0xff shr-int/lit8 v7, v7, 0x2 aget-byte v7, p1, v7 aput-byte v7, v6, v4 .line 145 add-int/lit8 v4, v3, 0x1 .end local v3 # "index":I .restart local v4 # "index":I aget-byte v7, p0, v1 and-int/lit8 v7, v7, 0x3 shl-int/lit8 v7, v7, 0x4 aget-byte v7, p1, v7 aput-byte v7, v6, v3 .line 146 add-int/lit8 v3, v4, 0x1 .end local v4 # "index":I .restart local v3 # "index":I aput-byte v9, v6, v4 .line 147 add-int/lit8 v4, v3, 0x1 .end local v3 # "index":I .restart local v4 # "index":I aput-byte v9, v6, v3 move v3, v4 .line 148 .end local v4 # "index":I .restart local v3 # "index":I goto :goto_64 .line 150 .end local v3 # "index":I .restart local v4 # "index":I :pswitch_8e add-int/lit8 v3, v4, 0x1 .end local v4 # "index":I .restart local v3 # "index":I aget-byte v7, p0, v1 and-int/lit16 v7, v7, 0xff shr-int/lit8 v7, v7, 0x2 aget-byte v7, p1, v7 aput-byte v7, v6, v4 .line 151 add-int/lit8 v4, v3, 0x1 .end local v3 # "index":I .restart local v4 # "index":I aget-byte v7, p0, v1 and-int/lit8 v7, v7, 0x3 shl-int/lit8 v7, v7, 0x4 add-int/lit8 v8, v1, 0x1 aget-byte v8, p0, v8 and-int/lit16 v8, v8, 0xff shr-int/lit8 v8, v8, 0x4 or-int/2addr v7, v8 aget-byte v7, p1, v7 aput-byte v7, v6, v3 .line 152 add-int/lit8 v3, v4, 0x1 .end local v4 # "index":I .restart local v3 # "index":I add-int/lit8 v7, v1, 0x1 aget-byte v7, p0, v7 and-int/lit8 v7, v7, 0xf shl-int/lit8 v7, v7, 0x2 aget-byte v7, p1, v7 aput-byte v7, v6, v4 .line 153 add-int/lit8 v4, v3, 0x1 .end local v3 # "index":I .restart local v4 # "index":I aput-byte v9, v6, v3 goto :goto_63 .line 158 .end local v4 # "index":I .restart local v3 # "index":I :catch_c2 move-exception v0 .line 159 .local v0, "e":Ljava/io/UnsupportedEncodingException; new-instance v7, Ljava/lang/AssertionError; invoke-direct {v7, v0}, Ljava/lang/AssertionError;->(Ljava/lang/Object;)V throw v7 .line 142 nop :pswitch_data_ca .packed-switch 0x1 :pswitch_6c :pswitch_8e .end packed-switch .end method ```

The bytecode generated by D8 looks correct to me. It's most likely not a bug in D8 itself. The bytecode just seems to be sufficiently different from the one generated by dx that it triggers a bug somewhere down the line.
DavidEdwards commented 6 years ago

@mtrakal Thank you for testing this. I can at least disable D8 until someone fixes that issue.

Was the D8 team notified of this issue? They can probably take it from here.

Edit: Looks like they were notified: https://issuetracker.google.com/issues/69364976

pavlus commented 6 years ago

It went out, that the problem was with MediaTek custom Art modifications:

https://issuetracker.google.com/issues/69364976#comment6

DavidEdwards commented 6 years ago

Lowering priority since there is nothing we can do in the dexer. This works with DX by accident because they generate the 'const 0' instruction earlier in the instruction stream. However, that doesn't fix the problem and the dex code is fine.

If that is the case, then the only viable option is disabling D8 compiling until Android 5.1 is no longer in demand, or the faulty Art modifications are rolled back / fixed? Is that something like is likely to happen?

JakeWharton commented 6 years ago

Very unlikely.

We might be able to massage the Java code statements ordering to workaround the bug now that we know the problematic dex and assembly pattern. It may not be possible to deviate far enough to actually avoid it, though.

On Thu, Nov 23, 2017, 8:11 AM David Edwards notifications@github.com wrote:

Lowering priority since there is nothing we can do in the dexer. This works with DX by accident because they generate the 'const 0' instruction earlier in the instruction stream. However, that doesn't fix the problem and the dex code is fine.

If that is the case, then the only viable option is disabling D8 compiling until Android 5.1 is no longer in demand https://chart.googleapis.com/chart?cht=p&chs=500x250&chl=Gingerbread%7CIce%20Cream%20Sandwich%7CJelly%20Bean%7CKitKat%7CLollipop%7CMarshmallow%7CNougat%7COreo&chd=t%3A0.5%2C0.5%2C6.2%2C13.8%2C27.2%2C30.9%2C20.6%2C0.3&chf=bg%2Cs%2C00000000&chco=c4df9b%2C6fad0c, or the faulty Art modifications are rolled back / fixed? Is that something like is likely to happen?

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/square/okio/issues/325#issuecomment-346614603, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEEEX4Ud282mW6jvTEhp4T_4shlScmEks5s5W8OgaJpZM4QbaVP .

swankjesse commented 6 years ago

@mtrakal good find! I’ve got one more favor to ask – can you rearrange Okio’s Java code to try to avoid the problems? Maybe add an extra local variable or something? If you can find something that works that doesn’t make the code much worse I’d love to merge that in.

guillaumewrobel commented 6 years ago

Any update?

madsager commented 6 years ago

We have landed a workaround for this bug in the custom MediaTek Art version. That workaround is finally available in Android Studio 3.1 canary 6 which is available for download now. I hope this solves these issues for everyone. If you still see these issues after compiling with 3.1 canary 6, please do file another bug report so we can look into it.

swankjesse commented 6 years ago

No plans for changes in Okio on this. If anyone can find a reliable fix that we can make, I’m interested.

anupdey99 commented 6 years ago

3.1.3 (June 2018) This update to Android Studio 3.1 includes fixes for the following bugs:

pavlus commented 6 years ago

@anupdey99 this thread is about crashes on some 5.1 devices which managed to dexopt (actually dex2oat), but weak verification introduced bugs in generated code, is this also fixed, anyone?

madsager commented 6 years ago

@pavlus @anupdey99 As far as we (the D8 team) knows, we have worked around all the VM bugs that were uncovered by D8 generated code for okio. So, yes, this issue has been worked around and should no longer happen. We were able to reproduce using 5.1 devices as well as 4.4 devices and our workarounds make our examples run without issues.

If anyone are still seeing issues with android studio 3.1.3 please do file bug reports at: https://issuetracker.google.com/issues/new?component=317603&template=1018721

Thanks! Mads