IssueTracker

Ian, assigning to you for now, feel free to reassign if someone else picks it up. There is a reproduction which is really nice, so we just have to figure out what is causing this.

FYI, this reproduces on dex2oat 5.1.1 to 9.0.0.

It looks like we are seeing this verification error now due to the exception live range change for b/119374410.
The DEX code for the method that fails verification is:

int[] com.clevertap.android.sdk.InAppFCManager.getInAppCountsFromPersistentStore(java.lang.String)
registers: 9, inputs: 2, outputs: 3
------------------------------------------------------------
inst#  offset  instruction         arguments
------------------------------------------------------------
         0x00, line 219, locals: [7 -> this, 8 -> inappID]
    0:   0x00: IgetObject          v0, v7, Field android.content.Context com.clevertap.android.sdk.InAppFCManager.context
    1:   0x02: ConstString         v1, "counts_per_inapp"
    2:   0x04: InvokeStatic        { v0 v1 } Lcom/clevertap/android/sdk/StorageHelper;->getPreferences(Landroid/content/Context;Ljava/lang/String;)Landroid/content/SharedPreferences;
    3:   0x07: MoveResultObject    v0
         0x08, line 220, locals: [0 -> prefs, 7 -> this, 8 -> inappID]
    4:   0x08: Const4              v1, 0x0 (0)
    5:   0x09: InvokeInterface     { v0 v8 v1 } Landroid/content/SharedPreferences;->getString(Ljava/lang/String;Ljava/lang/String;)Ljava/lang/String;
    6:   0x0c: MoveResultObject    v1
         0x0d, line 221, locals: [0 -> prefs, 1 -> str, 7 -> this, 8 -> inappID]
    7:   0x0d: Const4              v2, 0x2 (2)
    8:   0x0e: IfNez               v1, 0x16 (+8)
    9:   0x10: NewArray            v2, v2, int[]
   10:   0x12: FillArrayData       v2, 0x42 (+48)
   11:   0x15: ReturnObject        v2
         0x16, line 224, locals: [0 -> prefs, 1 -> str, 7 -> this, 8 -> inappID]
   12:   0x16: ConstString         v3, ","
   13:   0x18: InvokeVirtual       { v1 v3 } Ljava/lang/String;->split(Ljava/lang/String;)[Ljava/lang/String;
   14:   0x1b: MoveResultObject    v3
         0x1c, line 225, locals: [0 -> prefs, 1 -> str, 3 -> parts, 7 -> this, 8 -> inappID]
   15:   0x1c: ArrayLength         v4, v3
   16:   0x1d: IfEq                v4, v2, 0x25 (+8)
   17:   0x1f: NewArray            v2, v2, int[]
   18:   0x21: FillArrayData       v2, 0x4a (+41)
   19:   0x24: ReturnObject        v2
         0x25, line 228, locals: [0 -> prefs, 1 -> str, 3 -> parts, 7 -> this, 8 -> inappID]
   20:   0x25: NewArray            v4, v2, int[]
   21:   0x27: Const4              v5, 0x0 (0)
   22:   0x28: AgetObject          v6, v3, v5
   23:   0x2a: InvokeStatic        { v6 } Ljava/lang/Integer;->parseInt(Ljava/lang/String;)I
   24:   0x2d: MoveResult          v6
   25:   0x2e: Aput                v6, v4, v5
   26:   0x30: Const4              v5, 0x1 (1)
   27:   0x31: AgetObject          v6, v3, v5
   28:   0x33: InvokeStatic        { v6 } Ljava/lang/Integer;->parseInt(Ljava/lang/String;)I
   29:   0x36: MoveResult          v6
   30:   0x37: Aput                v6, v4, v5
   31:   0x39: ReturnObject        v4
         0x3a, line 228, locals: [0 -> prefs, 1 -> str, 3 -> parts, 7 -> this, 8 -> inappID]
         0x3a, line 229, locals: [0 -> prefs, 1 -> str, 7 -> this, 8 -> inappID]
   32:   0x3a: MoveException       v3
         0x3b, line 230, locals: [0 -> prefs, 1 -> str, 3 -> t, 7 -> this, 8 -> inappID]
   33:   0x3b: NewArray            v2, v2, int[]
   34:   0x3d: FillArrayData       v2, 0x52 (+21)
   35:   0x40: ReturnObject        v2
   36:   0x41: Nop                
   37:   0x42: Nop                 [FillArrayPayload], width: 4, size:  2
   38:   0x4a: Nop                 [FillArrayPayload], width: 4, size:  2
   39:   0x52: Nop                 [FillArrayPayload], width: 4, size:  2
Tries (numbers are offsets)
  [0x16 .. 0x38] -> 0
Handlers (numbers are offsets)
  0: [
       java.lang.Throwable -> 0x3a
     ]

The error here is that 0x3a has v2 marked as Conflict, but it really should be (and is) an Integer. Previous to the fix linked above, the v2 register would not be reused at 0x1f for the throwing new-array instruction. It appears that the verifier incorrectly propagates the register state from 0x21 (FillArrayData) or D8 is incorrect in assuming that FillArrayData cannot throw. I'll investigate if this is just related to FillArrayData or if this could be a more general issue.

This is an error in D8 that did not mark FillArrayData as throwing which is most definitely is and should be. This was hidden by the live range issue and by the fact that the construction of fill instructions is dealt with specially in the IRBuilder which reading in DEX. Fix is in the works.

Project: r8
Branch: master

commit faf0524459bb46c76a737798aaaa96a9b32efe06
Author: Ian Zerny <zerny@google.com>
Date:   Wed Jan 16 12:26:03 2019

    Correctly mark and treat FillArrayData as a throwing instruction.
   
    Bug: 122887884
    Change-Id: I99bd404c5b20d04881978fcc91b106fdec45d739

M       src/main/java/com/android/tools/r8/code/FillArrayData.java
M       src/main/java/com/android/tools/r8/ir/code/NewArrayFilledData.java
M       src/main/java/com/android/tools/r8/ir/conversion/DexSourceCode.java
M       src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
M       src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
A       src/test/java/com/android/tools/r8/regress/b122887884/Regress122887884.java
A       src/test/java/com/android/tools/r8/regress/b122887884/Regress122887884Runner.java

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

I can see that there has been a fix which is committed, is there an expected timeline for the same?

We need to merge the fix to release branches. Once we have done that we will post instructions on how to use the fixed version on this bug report until the next release of Android Studio 3.3.1. The instructions on how to use the fixed version will be posted here today.

Project: r8
Branch: d8-1.4

commit 0f046727ae838a980dcddeb777b0bcef0a28f9be
Author: Ian Zerny <zerny@google.com>
Date:   Wed Jan 16 13:13:50 2019

    Version 1.4.25
   
    Cherry pick: Correctly mark and treat FillArrayData as a throwing instruction.
    CL: https://r8-review.googlesource.com/c/r8/+/33037
    Bug: 122887884
   
    Cherry pick: Allow referencing graph nodes that are removed during tree shaking.
    CL: https://r8-review.googlesource.com/c/r8/+/32980
    Bug: 120959039
   
    Change-Id: Ic06dcf651f49f2c3fa7860155e1948ca43e7c95f

M       src/main/java/com/android/tools/r8/Version.java
M       src/main/java/com/android/tools/r8/code/FillArrayData.java
M       src/main/java/com/android/tools/r8/ir/code/NewArrayFilledData.java
M       src/main/java/com/android/tools/r8/ir/conversion/DexSourceCode.java
M       src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
M       src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
M       src/main/java/com/android/tools/r8/shaking/Enqueuer.java
A       src/test/java/com/android/tools/r8/regress/b122887884/Regress122887884.java
A       src/test/java/com/android/tools/r8/regress/b122887884/Regress122887884Runner.java
A       src/test/java/com/android/tools/r8/shaking/keptgraph/RemovedClassTest.java
A       src/test/java/com/android/tools/r8/shaking/keptgraph/RemovedClassTestRunner.java

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

Project: r8
Branch: d8-1.3

commit 33b7a220fef7d148d1e5de1734ef960a44e72020
Author: Ian Zerny <zerny@google.com>
Date:   Wed Jan 16 14:39:40 2019

    Version 1.3.52
   
    Cherry pick: Correctly mark and treat FillArrayData as a throwing instruction.
    CL: https://r8-review.googlesource.com/c/r8/+/33037
    Bug: 122887884
   
    Change-Id: I22fe243a80807d0ee8517243d1bc1590d2a19497

M       src/main/java/com/android/tools/r8/Version.java
M       src/main/java/com/android/tools/r8/code/FillArrayData.java
M       src/main/java/com/android/tools/r8/ir/code/NewArrayFilledData.java
M       src/main/java/com/android/tools/r8/ir/conversion/DexSourceCode.java
M       src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
M       src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
A       src/test/java/com/android/tools/r8/regress/b122887884/Regress122887884.java
A       src/test/java/com/android/tools/r8/regress/b122887884/Regress122887884Runner.java

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

The fix for this issue is available for AGP 3.3 (and 3.4) by setting an explicit dependency detailed below.
After AGP 3.3.1 is released remove the pinned version to allow you to pick up new D8/R8 releases again.

For AGP 3.3 amend your top-level build.gradle file with:

buildscript {
    repositories {
        maven {
            url "http://storage.googleapis.com/r8-releases/raw" // ADD THIS.
        }
    }
    dependencies {
        classpath 'com.android.tools:r8:1.3.52'  // ADD THIS. Must be before the Gradle Plugin for Android.
        classpath 'com.android.tools.build:gradle:3.3'
    }
}

(For AGP 3.4 the the r8 version should be 1.4.25)

Thank you so much for the quick turn around time for this issue!

I'm unable to test this specific version of of r8. It fails to find a transitive dependency kotlinx-metadata-jvm:

A problem occurred configuring root project 'android-libon-lite'.
> Could not resolve all artifacts for configuration ':classpath'.
   > Could not find org.jetbrains.kotlinx:kotlinx-metadata-jvm:0.0.3.
     Searched in the following locations:
       - http://storage.googleapis.com/r8-releases/raw/org/jetbrains/kotlinx/kotlinx-metadata-jvm/0.0.3/kotlinx-metadata-jvm-0.0.3.pom
       - http://storage.googleapis.com/r8-releases/raw/org/jetbrains/kotlinx/kotlinx-metadata-jvm/0.0.3/kotlinx-metadata-jvm-0.0.3.jar
       - https://jcenter.bintray.com/org/jetbrains/kotlinx/kotlinx-metadata-jvm/0.0.3/kotlinx-metadata-jvm-0.0.3.pom
       - https://jcenter.bintray.com/org/jetbrains/kotlinx/kotlinx-metadata-jvm/0.0.3/kotlinx-metadata-jvm-0.0.3.jar
       - https://dl.google.com/dl/android/maven2/org/jetbrains/kotlinx/kotlinx-metadata-jvm/0.0.3/kotlinx-metadata-jvm-0.0.3.pom
       - https://dl.google.com/dl/android/maven2/org/jetbrains/kotlinx/kotlinx-metadata-jvm/0.0.3/kotlinx-metadata-jvm-0.0.3.jar
       - https://repo.maven.apache.org/maven2/org/jetbrains/kotlinx/kotlinx-metadata-jvm/0.0.3/kotlinx-metadata-jvm-0.0.3.pom
       - https://repo.maven.apache.org/maven2/org/jetbrains/kotlinx/kotlinx-metadata-jvm/0.0.3/kotlinx-metadata-jvm-0.0.3.jar
       - https://plugins.gradle.org/m2/org/jetbrains/kotlinx/kotlinx-metadata-jvm/0.0.3/kotlinx-metadata-jvm-0.0.3.pom
       - https://plugins.gradle.org/m2/org/jetbrains/kotlinx/kotlinx-metadata-jvm/0.0.3/kotlinx-metadata-jvm-0.0.3.jar
     Required by:
         project : > com.android.tools:r8:1.3.52

You can see it is looking in the additional maven repo. I also added jcenter, google, and mavencentral repos (normally we just have our own private nexus mirror which i've removed from the above snippet)

Try adding the repo `maven { url “https://kotlin.bintray.com/kotlinx/” }`.

Actually I found that excluding transitive dependencies worked:

classpath ('com.android.tools:r8:1.3.52' ) { transitive false }