Backport changes that made D8 lenient with non-sense line numbers and local variable table [156142802]

Fixed

Bug

Status Update

No update yet.

Description

js...@google.com

created issue #1

May 9, 2020 08:18AM

Note that this issue is not reproduced on master, but still present up to the latest stable version, 2.0.73. That's why this issue is about figuring out when and how things are fixed, and if possible, backporting them to necessary stable branches, e.g., 2.0 (for AS 4.0).

The issue is found while working on a new front-end (a.k.a. fir) of kotlinc. Please find attached class file generated by (still work-in-progress) fir. Please note that the class file has a resolution error, so the expected behavior is:

$ java -cp .:out.jar main
Exception in thread "main" java.lang.NoSuchMethodError: kotlin.ranges.RangesKt.contains(Lkotlin/ranges/ClosedRange;J)Z
	at InRangeWithNonmatchingArgumentsKt.box(inRangeWithNonmatchingArguments.kt)
	at main.main(main.java:3)

if the main is simply calling InRangeWithNonmatchingArgumentsKt#box.

Interestingly, depending on versions, errors are different, but I guess it's just due to the different place of "first" isConsistentSSA check.

$ git checkout 2.0.73
$ ./tools/d8.py ~/tmp/InRangeWithNonmatchingArgumentsKt.class --output ~/tmp
...
Caused by: java.lang.AssertionError
	at com.android.tools.r8.ir.code.IRCode.consistentDefUseChains(IRCode.java:650)
	at com.android.tools.r8.ir.code.IRCode.isConsistentSSA(IRCode.java:549)
	at com.android.tools.r8.ir.desugar.LambdaRewriter.desugarLambdas(LambdaRewriter.java:278)
	at com.android.tools.r8.ir.conversion.IRConverter.optimize(IRConverter.java:1139)
	at com.android.tools.r8.ir.conversion.IRConverter.rewriteCodeInternal(IRConverter.java:1100)
	at com.android.tools.r8.ir.conversion.IRConverter.lambda$rewriteCode$12(IRConverter.java:1072)
	at com.android.tools.r8.utils.ExceptionUtils.withOriginAttachmentHandler(ExceptionUtils.java:141)
	at com.android.tools.r8.ir.conversion.IRConverter.rewriteCode(IRConverter.java:1068)
	at com.android.tools.r8.ir.conversion.IRConverter.convertMethod(IRConverter.java:608)
	at com.android.tools.r8.ir.conversion.IRConverter.lambda$convertMethods$3(IRConverter.java:567)
	at com.android.tools.r8.graph.DexClass.forEachMethod(DexClass.java:346)
	at com.android.tools.r8.ir.conversion.IRConverter.convertMethods(IRConverter.java:563)
	at com.android.tools.r8.utils.ThreadUtils.lambda$processItems$0(ThreadUtils.java:25)
	at java.base/java.util.concurrent.ForkJoinTask$AdaptedCallable.exec(ForkJoinTask.java:1431)
	at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:283)
	at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1603)
	at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:175)

$ git checkout 1.6.89
$ ./tools/d8.py ~/tmp/InRangeWithNonmatchingArgumentsKt.class --output ~/tmp
...
Caused by: java.lang.AssertionError
	at com.android.tools.r8.ir.code.IRCode.consistentDefUseChains(IRCode.java:602)
	at com.android.tools.r8.ir.code.IRCode.isConsistentSSA(IRCode.java:501)
	at com.android.tools.r8.ir.optimize.CodeRewriter.rewriteKnownArrayLengthCalls(CodeRewriter.java:3753)
	at com.android.tools.r8.ir.conversion.IRConverter.rewriteCodeInternal(IRConverter.java:1226)
	at com.android.tools.r8.ir.conversion.IRConverter.rewriteCode(IRConverter.java:1044)
	at com.android.tools.r8.ir.conversion.IRConverter.convertMethod(IRConverter.java:605)
	at com.android.tools.r8.ir.conversion.IRConverter.lambda$convertMethods$4(IRConverter.java:564)
	at com.android.tools.r8.graph.DexClass.forEachMethod(DexClass.java:294)
	at com.android.tools.r8.ir.conversion.IRConverter.convertMethods(IRConverter.java:560)
	at com.android.tools.r8.ir.conversion.IRConverter.lambda$convertClasses$3(IRConverter.java:545)
	at java.base/java.util.concurrent.ForkJoinTask$AdaptedRunnableAction.exec(ForkJoinTask.java:1393)
	at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:283)
	at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1603)
	at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:175)

And, this is the version used by kotlin project (I'll update that once the original issue is fixed; I don't want to update D8/R8 to hide the other issue):

$ git checkout 1.5.70
$ ./tools/d8.py ~/tmp/InRangeWithNonmatchingArgumentsKt.class --output ~/tmp
...
Caused by: com.android.tools.r8.utils.AbortException: Error: java.lang.AssertionError
	at com.android.tools.r8.ir.code.IRCode.consistentDefUseChains(IRCode.java:551)
	at com.android.tools.r8.ir.code.IRCode.isConsistentSSA(IRCode.java:470)
	at com.android.tools.r8.ir.optimize.CodeRewriter.commonSubexpressionElimination(CodeRewriter.java:2913)
	at com.android.tools.r8.ir.conversion.IRConverter.rewriteCode(IRConverter.java:987)
	at com.android.tools.r8.ir.conversion.IRConverter.convertMethodToDex(IRConverter.java:486)
	at com.android.tools.r8.ir.conversion.IRConverter.lambda$convertMethodsToDex$3(IRConverter.java:474)
	at com.android.tools.r8.graph.DexClass.forEachMethod(DexClass.java:291)
	at com.android.tools.r8.ir.conversion.IRConverter.convertMethodsToDex(IRConverter.java:470)
	at com.android.tools.r8.ir.conversion.IRConverter.lambda$convertClassesToDex$2(IRConverter.java:455)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)
	Suppressed: java.util.concurrent.ExecutionException: java.lang.AssertionError
		at java.util.concurrent.FutureTask.report(FutureTask.java:122)
		at java.util.concurrent.FutureTask.get(FutureTask.java:192)
		at com.android.tools.r8.utils.ThreadUtils.awaitFutures(ThreadUtils.java:21)
		at com.android.tools.r8.ir.conversion.IRConverter.convertClassesToDex(IRConverter.java:457)
		at com.android.tools.r8.ir.conversion.IRConverter.convertToDex(IRConverter.java:364)
		at com.android.tools.r8.D8.optimize(D8.java:230)
		at com.android.tools.r8.D8.run(D8.java:171)
		at com.android.tools.r8.D8.lambda$run$1(D8.java:98)
		at com.android.tools.r8.utils.ExceptionUtils.withCompilationHandler(ExceptionUtils.java:61)
		at com.android.tools.r8.utils.ExceptionUtils.withD8CompilationHandler(ExceptionUtils.java:44)
		at com.android.tools.r8.D8.run(D8.java:95)

InRangeWithNonmatchingArgumentsKt.class

2.3 KB

Download

Comments

sg...@google.com <sg...@google.com> May 11, 2020 07:33AM

Reassigned to cl...@google.com.

cl...@google.com <cl...@google.com> #2May 11, 2020 12:33PM

We changed the place of "isConsistentSSA" recently. The reason was that the IR is not necessarily in consistent SSA at the beginning of IR processing, until the lensCodeRewriter, enum unboxer rewriter and lambda merger have been fully run. We moved the check down so that the IR is necessarily in consistent SSA when the check is performed. It may have been moved twice (once to make enum unboxing work, a second time as a refactoring to make the code nice and clean).

We still need to investigate why the def-use chain is broken though.

cl...@google.com <cl...@google.com> #3May 14, 2020 08:18AM

~~We cannot backport anymore to 4.0.~~

I will try to bisect to figure out where does this come from.

Message last modified on May 15, 2020 07:23AM

cl...@google.com <cl...@google.com> #4May 14, 2020 02:25PM

The commit below is responsible for the fix. ~~I don't think we can backport to 4.0 at this point however.~~

Author: Ian Zerny <zerny@google.com>
Date:   Mon Feb 3 12:50:59 2020 +0100

    Don't replace local writes by phi introduced locals.
    
    Bug: 148661132
    Change-Id: Iab815abe01e6db2c7b9bdf86b2e0715d7a5a2a4f

 .../android/tools/r8/ir/optimize/CodeRewriter.java |  32 -------
 .../tools/r8/debug/Regress148661132Test.java       | 100 +++++++++++++++++++++
 2 files changed, 100 insertions(+), 32 deletions(-)
 create mode 100644 src/test/java/com/android/tools/r8/debug/Regress148661132Test.java

Message last modified on May 15, 2020 07:24AM

cl...@google.com <cl...@google.com> #5May 15, 2020 07:23AM

OK I'm making 2.0.74 and backporting this fix.

ap...@google.com <ap...@google.com> #6May 15, 2020 11:21AM

Project: r8
Branch: 2.0

commit 685ad9374465eb129da58ec98866fb293828285e
Author: Clément Béra <clementbera@google.com>
Date: Fri May 15 07:09:38 2020

Version 2.0.74

Cherry-pick: Don't replace local writes by phi introduced locals.
CL:

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

Bug: 148661132
Bug: 156142802
Change-Id: Iab815abe01e6db2c7b9bdf86b2e0715d7a5a2a4f

M src/main/java/com/android/tools/r8/Version.java
M src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
A src/test/java/com/android/tools/r8/debug/Regress148661132Test.java

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

cl...@google.com <cl...@google.com> #7May 18, 2020 06:34AM

Marked as fixed.

https://googleplex-android-review.git.corp.google.com/c/platform/prebuilts/r8/+/11520859

an...@google.com <an...@google.com> #8May 20, 2020 02:58PM

The following changes were cherrypicked through Coastguard (

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

https://android-build.googleplex.com/coastguard/dashboard/6387293562667008/#/history/all)
Created: 2020-05-20 14:56:08.316344+00:00
Changes: ag/11520859

to...@gmail.com <to...@gmail.com> #9May 22, 2020 08:57AM

Just in case posting here too as 2.0.X is RC: https://issuetracker.google.com/issues/157223339

This have cause a regression and important bug with coroutines.

sg...@google.com <sg...@google.com> #10May 27, 2020 08:50AM

Just for the record. The issue b/157223339 is not related to this change (2.0.74 vs. 2.0.73). The b/157223339 issue is also in 2.0.73.