Bug in PeepholeOptimizer [148366506]

Fixed

Bug

Reviewed (L1)

Status Update

No update yet.

Description

[Deleted User]

created issue #1

Jan 27, 2020 02:48PM

I came across a bug in d8 which produces the exception below, when in release mode only. I traced the cause to the PeepholeOptimizer, specifically line 105.

java -jar d8.jar --output myout.jar --no-desugaring --release storagehelper-out.jar

In the CFG, two blocks with the same successor share the same prefix (a const-string instruction) and each of those blocks have catch handlers.

The optimizer decides that it can share the const-string instructions because it checks instructionInstanceCanThrow() instead of instructionTypeCanThrow().

I changed that line and it fixes the problem. From my limited understanding of D8 this seems to work but maybe it's not the correct/only fix to make.

com.android.tools.r8.errors.Unreachable: Expected to find a possibly throwing instruction
at com.android.tools.r8.ir.conversion.DexBuilder.trimEnd(DexBuilder.java:835)
at com.android.tools.r8.ir.conversion.DexBuilder.computeTryItems(DexBuilder.java:796)
at com.android.tools.r8.ir.conversion.DexBuilder.computeTryInfo(DexBuilder.java:739)
at com.android.tools.r8.ir.conversion.DexBuilder.build(DexBuilder.java:307)
at com.android.tools.r8.graph.DexEncodedMethod.setCode(DexEncodedMethod.java:572)
at com.android.tools.r8.ir.conversion.IRConverter.finalizeToDex(IRConverter.java:1610)
at com.android.tools.r8.ir.conversion.IRConverter.finalizeIR(IRConverter.java:1581)
at com.android.tools.r8.ir.conversion.IRConverter.optimize(IRConverter.java:1543)
at com.android.tools.r8.ir.conversion.IRConverter.rewriteCodeInternal(IRConverter.java:1071)
at com.android.tools.r8.ir.conversion.IRConverter.rewriteCode(IRConverter.java:1035)
at com.android.tools.r8.ir.conversion.IRConverter.convertMethod(IRConverter.java:600)
at com.android.tools.r8.ir.conversion.IRConverter.lambda$convertMethods$3(IRConverter.java:559)
at com.android.tools.r8.ir.conversion.IRConverter$$Lambda$170.00000000B8025B40.accept(Unknown Source)
at com.android.tools.r8.graph.DexClass.forEachMethod(DexClass.java:310)
at com.android.tools.r8.ir.conversion.IRConverter.convertMethods(IRConverter.java:555)
at com.android.tools.r8.ir.conversion.IRConverter$$Lambda$167.000000002C735750.accept(Unknown Source)
at com.android.tools.r8.utils.ThreadUtils.lambda$processItems$1(ThreadUtils.java:36)
at com.android.tools.r8.utils.ThreadUtils$$Lambda$168.000000002C735BF0.apply(Unknown Source)
at com.android.tools.r8.utils.ThreadUtils.lambda$processItemsWithResults$0(ThreadUtils.java:25)
at com.android.tools.r8.utils.ThreadUtils$$Lambda$169.000000002C736880.call(Unknown Source)
at java.util.concurrent.ForkJoinTask$AdaptedCallable.exec(ForkJoinTask.java:1424)
at java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:289)
at java.util.concurrent.ForkJoinPool$WorkQueue.runTask(ForkJoinPool.java:1056)
at java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1692)
at java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:157)

fix.patch

871 B

View

Download

storagehelper-out.jar

1.2 KB

Download

Comments

sg...@google.com <sg...@google.com> Jan 28, 2020 07:40AM

Reassigned to sg...@google.com.

ap...@google.com <ap...@google.com> #2Feb 3, 2020 11:32AM

Project: r8
Branch: master

commit 9dbee069fc6e04c60b155929d5a83cda30d20326
Author: Søren Gjesse <sgjesse@google.com>
Date: Mon Feb 03 10:42:41 2020

Don't use instructionInstanceCanThrow in the peep hole optimizer

* This can lead to empty blocks with catch handlers, which the
pipeline following the peep hole optimizer assumes cannot happen
* This could move an instruction which - according to the Art/Dalvik
verifier - can throw outside a catch handler possbily causing a
verification error

Bug: 148366506
Change-Id: I0d7c23507836144f789b5adbd5363f3ee0cd9a4b

M src/main/java/com/android/tools/r8/ir/optimize/PeepholeOptimizer.java
A src/test/java/com/android/tools/r8/ir/optimize/B148366506.java

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

sg...@google.com <sg...@google.com> Feb 3, 2020 11:32AM

Marked as fixed.

ap...@google.com <ap...@google.com> #3Feb 3, 2020 02:30PM

Project: r8
Branch: 2.0

commit 8779fadfb53fc95e47452b6122690c83f86b741b
Author: Søren Gjesse <sgjesse@google.com>
Date: Mon Feb 03 15:21:15 2020

Version 2.0.26

Don't use instructionInstanceCanThrow in the peep hole optimizer
CL:

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

Bug: 148366506
Change-Id: I280f5818de8cebacd60e54db1db77888ac7d67f7

M src/main/java/com/android/tools/r8/Version.java
M src/main/java/com/android/tools/r8/ir/optimize/PeepholeOptimizer.java
A src/test/java/com/android/tools/r8/ir/optimize/B148366506.java

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

ap...@google.com <ap...@google.com> #4Feb 4, 2020 08:02AM

Project: r8
Branch: 1.6

commit e4f7acb2bdbbc8ab5334e101bf20c085c68060fb
Author: Søren Gjesse <sgjesse@google.com>
Date: Mon Feb 03 15:22:50 2020

Version 1.6.68

Don't use instructionInstanceCanThrow in the peep hole optimizer
CL:

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

Bug: 148366506
Change-Id: Ie4e4572cfbc60d44a89e18601ea0cf93cadbb9fd

M src/main/java/com/android/tools/r8/Version.java
M src/main/java/com/android/tools/r8/ir/optimize/PeepholeOptimizer.java
A src/test/java/com/android/tools/r8/ir/optimize/B148366506.java

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

Issue 148366506