Status Update
Comments
bi...@mail.de <bi...@mail.de> #2
The make-proguard-happy module just adds an annotation of android that was missing.
sg...@google.com <sg...@google.com>
mk...@google.com <mk...@google.com> #3
Thank you for the reproduction and I am able to reproduce it. The error only happens in debug-mode. Will see if I can figure out what causes the invalid frame and make a fix starting next week.
mk...@google.com <mk...@google.com> #4
OK, I was able to figure out what the problem is. This is part of the original code:
215: goto L43
216: L37: ; locals: 0 => this, 1 => $result, 3 => $this$consume$iv, 5 => $i$f$consume, 6 => cause$iv
217: .line 152
218: ; frame: [0:io/ktor/client/engine/java/JavaHttpResponseBodyHandler$JavaHttpResponseBodySubscriber$1, 1:java/lang/Object, 2:kotlinx/coroutines/CoroutineScope, 3:kotlinx/coroutines/channels/ReceiveChannel, 4:io/ktor/client/engine/java/JavaHttpResponseBodyHandler$JavaHttpResponseBodySubscriber, 5:int, 6:null, 7:oneword, 8:oneword, 9:oneword, 10:java/lang/Object] [java/lang/Throwable]
219: astore 9
220: L38: ; locals: 0 => this, 1 => $result, 3 => $this$consume$iv, 5 => $i$f$consume, 6 => cause$iv, 9 => e$iv
221: .line 153
222: aload 9
223: astore 6
224: L39: ; locals: 0 => this, 1 => $result, 3 => $this$consume$iv, 5 => $i$f$consume, 6 => cause$iv, 9 => e$iv
225: .line 154
226: aload 9
227: athrow
228: L40: ; locals: 0 => this, 1 => $result, 3 => $this$consume$iv, 5 => $i$f$consume, 6 => cause$iv
229: .line 155
230: ; frame: [0:io/ktor/client/engine/java/JavaHttpResponseBodyHandler$JavaHttpResponseBodySubscriber$1, 1:java/lang/Object, 2:kotlinx/coroutines/CoroutineScope, 3:kotlinx/coroutines/channels/ReceiveChannel, 4:io/ktor/client/engine/java/JavaHttpResponseBodyHandler$JavaHttpResponseBodySubscriber, 5:int, 6:java/lang/Throwable, 7:oneword, 8:oneword, 9:oneword, 10:java/lang/Object] [java/lang/Throwable]
The athrow
on 227 will delegate to the catch handler at 228.
This is our emitted code:
401: L91: ; locals: 0 => this, 1 => $result, 3 => $i$f$consume, 5 => cause$iv, 7 => $this$consume$iv
402: ; frame: [0:io/ktor/client/engine/java/JavaHttpResponseBodyHandler$JavaHttpResponseBodySubscriber$1, 1:java/lang/Object, 2:java/lang/Throwable, 3:int, 5:null, 7:java/lang/Object] []
403: L92: ; locals: 0 => this, 1 => $result, 3 => $i$f$consume, 5 => cause$iv, 7 => $this$consume$iv
404: .line 152
405: aload 2
406: astore 2
407: L93: ; locals: 0 => this, 1 => $result, 2 => e$iv, 3 => $i$f$consume, 5 => cause$iv, 7 => $this$consume$iv
408: .line 153
409: aload 2
410: astore 4
411: L94: ; locals: 0 => this, 1 => $result, 2 => e$iv, 3 => $i$f$consume, 4 => cause$iv, 7 => $this$consume$iv
412: .line 154
413: aload 2
414: L95: ; locals: 2 => e$iv
415: athrow
416: L96: ; locals: 0 => this, 1 => $result, 3 => $i$f$consume, 4 => cause$iv, 7 => $this$consume$iv
417: ; frame: [0:io/ktor/client/engine/java/JavaHttpResponseBodyHandler$JavaHttpResponseBodySubscriber$1, 1:java/lang/Object, 3:int, 4:java/lang/Throwable, 7:java/lang/Object] [java/lang/Throwable]
Here the athrow
is on 415 and the catch block is directly below on 416.
It is a bit subtle, because in our code we pass the throwable in a register (2) and it is not on the stack as it is in the original. Anyway, the assumption that we have is that stores (astore
, istore
) are not throwing instructions - that is, there is no need to check exception edges. So the astore 2
on 406 should not have a frame that matches the frame on 417. However, that is not what the VM does, it checks stores and that is why you get the error you get, because the current frame on 406 is NOT assignable to the frame on 417. Updating our stack map validation to check stores is tracked by a new bug here:
In this case the reason why this is not working is because of the shift in register position for cause$iv
. In our code we swap it from register 5 to register 4. In the original code it stays on 6. The local range for cause$iv
is split on the input:
.var 6 is cause$iv Ljava/lang/Throwable; from L37 to L39
.var 9 is e$iv Ljava/lang/Throwable; from L38 to L40
.var 6 is cause$iv Ljava/lang/Throwable; from L39 to L42
For some reason our register allocation then changes the position of locals on what corresponds to L39 in our code.
This is not a minor change and require a discussion on how to proceed best so I do not have any good fix for you except to use release mode or continue to use proguard. We will discuss this when most people are back around the start of August.
bi...@mail.de <bi...@mail.de> #5
Ok, thanks for the update! I'll try to test without --debug
in the next days, it's just there because I found that elsewhere 🤷.
I'm just wondering, I can't be the first one to use r8 and ktor. Is this something that does not appear on android?
mk...@google.com <mk...@google.com> #6
Yes, stack map verification is only done when running on the JVM, not even by javac. Normally all code passed through D8/R8 is for running on ART.
ap...@google.com <ap...@google.com> #7
Branch: main
commit 77a6d243cf1e02beda644d5896fc789c9964d918
Author: Morten Krogh-Jespersen <mkroghj@google.com>
Date: Thu Aug 04 09:37:46 2022
Add test for invalid stackmap for debug locals and trycatch
Bug:
Change-Id: I0edc7aa8bd827db46cae428e48964c47a99165f0
A src/test/java/com/android/tools/r8/cf/CfDebugLocalStackMapVerificationTest.java
mk...@google.com <mk...@google.com>
ap...@google.com <ap...@google.com> #8
Branch: main
commit fe051981a30c1ed914955d18e1d310cd43b76a3c
Author: Ian Zerny <zerny@google.com>
Date: Fri Aug 05 12:14:23 2022
Small regression test for invalid exceptional transfers.
Bug:
Change-Id: I8ac44f7df25f0a0ca28ed8a02c4a2a2946043bd9
M src/main/java/com/android/tools/r8/cf/CfRegisterAllocator.java
M src/test/java/com/android/tools/r8/cf/CfDebugLocalStackMapVerificationTest.java
M src/main/java/com/android/tools/r8/utils/InternalOptions.java
ap...@google.com <ap...@google.com> #9
Branch: main
commit f787c4dbcf81efc7594560aa1665434ff55f0ed4
Author: Ian Zerny <zerny@google.com>
Date: Mon Aug 08 15:42:48 2022
Split block after implicit store via DebugLocalWrite.
Fixes:
Change-Id: I6023e442a97838dfa816ad778b49a96d99b608ac
M src/main/java/com/android/tools/r8/ir/code/DebugLocalWrite.java
M src/main/java/com/android/tools/r8/cf/LoadStoreHelper.java
M src/test/java/com/android/tools/r8/cf/CfDebugLocalStackMapVerificationTest.java
ze...@google.com <ze...@google.com> #10
This should be cherry picked to release branches too.
ap...@google.com <ap...@google.com> #11
Branch: 3.2
commit c1f5a649e6b174dc5c977090c70d2cb0cc4caf58
Author: Ian Zerny <zerny@google.com>
Date: Tue Aug 09 09:32:15 2022
Version 3.2.77
Bug:
Change-Id: I03b64963a474c2ad8709fffe6349769c8788df41
M src/main/java/com/android/tools/r8/Version.java
ap...@google.com <ap...@google.com> #12
Branch: 3.2
commit 0d5e3c0c6516de5047c1747fda8ea5c5ac212bdf
Author: Ian Zerny <zerny@google.com>
Date: Tue Aug 09 09:30:51 2022
Split block after implicit store via DebugLocalWrite.
Bug:
Change-Id: I6023e442a97838dfa816ad778b49a96d99b608ac
M src/main/java/com/android/tools/r8/ir/code/DebugLocalWrite.java
M src/main/java/com/android/tools/r8/cf/LoadStoreHelper.java
M src/test/java/com/android/tools/r8/cf/CfDebugLocalStackMapVerificationTest.java
ap...@google.com <ap...@google.com> #13
Branch: 3.2
commit 1cdabd9d14c033400d9bcfeb40b720523500283a
Author: Ian Zerny <zerny@google.com>
Date: Tue Aug 09 09:06:42 2022
Small regression test for invalid exceptional transfers.
Bug:
Change-Id: I8ac44f7df25f0a0ca28ed8a02c4a2a2946043bd9
M src/main/java/com/android/tools/r8/cf/CfRegisterAllocator.java
M src/test/java/com/android/tools/r8/cf/CfDebugLocalStackMapVerificationTest.java
M src/main/java/com/android/tools/r8/utils/InternalOptions.java
ap...@google.com <ap...@google.com> #14
Branch: 3.2
commit 028f9e7d5b2a94b8bc8e3ba1f2c0f3eecc795564
Author: Ian Zerny <zerny@google.com>
Date: Tue Aug 09 08:55:57 2022
Add test for invalid stackmap for debug locals and trycatch
Bug:
Change-Id: I0edc7aa8bd827db46cae428e48964c47a99165f0
A src/test/java/com/android/tools/r8/cf/CfDebugLocalStackMapVerificationTest.java
ap...@google.com <ap...@google.com> #15
Branch: 3.3
commit b46c3d970800b208b33ee6506d167444c434c7fb
Author: Ian Zerny <zerny@google.com>
Date: Tue Aug 09 12:32:58 2022
Version 3.3.72
Bug:
Change-Id: Ie4d364afbb9fa5fbc3d9e0a691747d3988351aec
M src/main/java/com/android/tools/r8/Version.java
ap...@google.com <ap...@google.com> #16
Branch: 3.3
commit 1cbd003b35f979a6c95ed13ccf03922ac32bf883
Author: Ian Zerny <zerny@google.com>
Date: Tue Aug 09 12:19:24 2022
Split block after implicit store via DebugLocalWrite.
Bug:
Change-Id: I6023e442a97838dfa816ad778b49a96d99b608ac
M src/main/java/com/android/tools/r8/ir/code/DebugLocalWrite.java
M src/main/java/com/android/tools/r8/cf/LoadStoreHelper.java
M src/test/java/com/android/tools/r8/cf/CfDebugLocalStackMapVerificationTest.java
ap...@google.com <ap...@google.com> #17
Branch: 3.3
commit d86c058a65371a93762a10ee81b028e987c11263
Author: Ian Zerny <zerny@google.com>
Date: Tue Aug 09 12:19:13 2022
Small regression test for invalid exceptional transfers.
Bug:
Change-Id: I8ac44f7df25f0a0ca28ed8a02c4a2a2946043bd9
M src/main/java/com/android/tools/r8/cf/CfRegisterAllocator.java
M src/test/java/com/android/tools/r8/cf/CfDebugLocalStackMapVerificationTest.java
M src/main/java/com/android/tools/r8/utils/InternalOptions.java
ap...@google.com <ap...@google.com> #18
Branch: 3.3
commit ee6fe4992c5267d5dfc3e9b178dcacb4de66bfc8
Author: Ian Zerny <zerny@google.com>
Date: Tue Aug 09 12:19:01 2022
Add test for invalid stackmap for debug locals and trycatch
Bug:
Change-Id: I0edc7aa8bd827db46cae428e48964c47a99165f0
A src/test/java/com/android/tools/r8/cf/CfDebugLocalStackMapVerificationTest.java
ze...@google.com <ze...@google.com> #19
Thanks for reporting this issue. The fix for it has landed on our main branch and release branches for 3.2 and 3.3.
You can locally use a newer version of R8 by updating your settings.gradle (or settings.gradle.kts) file with the below template:
pluginManagement {
buildscript {
repositories {
mavenCentral()
maven {
url = uri("https://storage.googleapis.com/r8-releases/raw")
}
}
dependencies {
classpath("com.android.tools:r8:<r8version>") // This should be 3.2.77 for AGP 7.2 or 3.3.72 for AGP 7.3
}
}
}
ap...@google.com <ap...@google.com> #20
Branch: 3.3
commit 77bf094c51e2ca9398e3ba36030efa8c2f923e1a
Author: Morten Krogh-Jespersen <mkroghj@google.com>
Date: Wed Aug 10 15:06:47 2022
Version 3.3.74
Bug:
Change-Id: I1251991627fa1028c60698aa2f5cfa5b75076c9a
M src/main/java/com/android/tools/r8/Version.java
bi...@mail.de <bi...@mail.de> #21
Thank you for fixing!
bi...@mail.de <bi...@mail.de> #22
What about
Description
When I shrink my shadow jar with r8, r8 messes up. I created a sample project where r8 is told not to do anything, still the jar contains invalid bytecode, leading to the VerifyError:https://github.com/binarynoise/ktor-test
As far as I can tell, my toolchain is up-to-date. Also happens with the CIO implemenatation of Ktor. As the not-minified shadow jar works as intended, I suspect r8 to be buggy.
This is my build log: