Status Update
No update yet.
Comments
sg...@google.com
sg...@google.com
Thank you for your patience while our engineering team worked to resolve this issue. A fix for this issue is now available in:
- Android Studio Ladybug Feature Drop | 2024.2.2 Canary 3
- Android Gradle Plugin 8.8.0-alpha03
We encourage you to try the latest update.
If you notice further issues or have questions, please file a new bug report.
Thank you for taking the time to submit feedback — we really appreciate it!
sg...@google.com
Status of current findings.
This is related to the SplitReturnRewriter added in
Not running this code pass on java.lang.Object androidx.collection.LruCache.remove(java.lang.Object) works around the issue. I used the following patch:
diff --git a/src/main/java/com/android/tools/r8/ir/conversion/passes/SplitReturnRewriter.java b/src/main/java/com/android/tools/r8/ir/conversion/passes/SplitReturnRewriter.java
index 157d26c66a..63e0f76064 100644
--- a/src/main/java/com/android/tools/r8/ir/conversion/passes/SplitReturnRewriter.java
+++ b/src/main/java/com/android/tools/r8/ir/conversion/passes/SplitReturnRewriter.java
@@ -43,6 +43,10 @@ public class SplitReturnRewriter extends CodeRewriterPass<AppInfo> {
@Override
protected CodeRewriterResult rewriteCode(IRCode code) {
+ if (code.context().toSourceString().equals(
+ "java.lang.Object androidx.collection.LruCache.remove(java.lang.Object)")) {
+ return CodeRewriterResult.NO_CHANGE;
+ }
int color = colorExceptionHandlers(code);
AffectedValues affectedValues = new AffectedValues();
boolean changed = false;
Comparing the DEX with and without this patch shows one instruction diff in void androidx.leanback.widget.GridLayoutManager$2.addItem(java.lang.Object, int, int, int, int) where java.lang.Object androidx.collection.LruCache.remove(java.lang.Object) is inlined into.
Tracing the IR for void androidx.leanback.widget.GridLayoutManager$2.addItem(java.lang.Object, int, int, int, int) for the two versions shows that they start to deviate in second pass of IR after inserting assume instructions (SSA) with
block 29, pred-counts: 1, succ-count: 2, filled: true, sealed: false
predecessors: 28
successors: 31 30 (no try/catch successors)
no phis
#143:loadView;1762:addItem: -1: InstanceGet v67 <- v136; field: androidx.collection.LruCache androidx.leanback.widget.ViewsStateBundle.mChildStates
: -1: Assume v140 <- v67; lower bound: @Nullable androidx.collection.LruCache {}
: -1: SafeCheckCast v131 <- v65; java.lang.String
: -1: Invoke-Virtual v132 <- v140, v131; method: java.lang.Object androidx.collection.LruCache.remove(java.lang.String)
: -1: CheckCast v69 <- v132; android.util.SparseArray
#144:loadView;1762:addItem: -1: If v69 EQZ block 31 (fallthrough 30)
vs.
block 29, pred-counts: 1, succ-count: 2, filled: true, sealed: false
predecessors: 28
successors: 31 30 (no try/catch successors)
no phis
#143:loadView;1762:addItem: -1: InstanceGet v67 <- v136; field: androidx.collection.LruCache androidx.leanback.widget.ViewsStateBundle.mChildStates
: -1: Assume v140 <- v67; lower bound: @Nullable androidx.collection.LruCache {}
: -1: SafeCheckCast v131 <- v65; java.lang.String
: -1: Invoke-Virtual v132 <- v140, v131; method: java.lang.Object androidx.collection.LruCache.remove(java.lang.String)
: -1: Assume v141 <- v132; not null
: -1: CheckCast v69 <- v141; android.util.SparseArray
#144:loadView;1762:addItem: -1: If v69 EQZ block 31 (fallthrough 30)
This is from computeDynamicReturnType concluding a dynamic return type of DynamicTypeWithUpperBound(@NotNull java.lang.Object {}).
sg...@google.com
Looks like dead code removal in java.lang.Object androidx.collection.LruCache.remove(java.lang.Object) is causing this.
The method java.lang.Object androidx.collection.LruCache.remove(java.lang.Object) is processed twice in first round of IR, second time with more precise type for the argument as java.lang.Object androidx.collection.LruCache.remove(java.lang.String).
That second processing involves quite a bit of inlining. However before dead code removal the return is still split between null and not null return value:
block 33, pred-counts: 1, succ-count: 2, filled: true, sealed: false
predecessors: 32
successors: 37 34 (no try/catch successors)
no phis
#187:remove;0:remove: -1: If v54 EQZ block 37 (fallthrough 34)
block 34, pred-counts: 1, succ-count: 1, filled: true, sealed: false
predecessors: 33
successors: 35 (no try/catch successors)
no phis
#187:remove;0:remove: -1: Assume v49 <- v54; not null
#188:remove;0:remove: -1: ConstNumber v32(0b0) <- 0 (INT)
: -1: ConstNumber v33(0) <- 0 (@Null NULL)
: -1: Goto block 35
block 35, pred-counts: 1, succ-count: 0, filled: true, sealed: false
predecessors: 34
successors: -
no phis
#188:remove;0:remove: -1: ConstString v90 <- "key"
: -1: Invoke-Static v45, v90; method: void kotlin.jvm.internal.Intrinsics.checkNotNullParameter(java.lang.Object, java.lang.String)
: -1: ConstString v91 <- "oldValue"
: -1: Invoke-Static v49, v91; method: void kotlin.jvm.internal.Intrinsics.checkNotNullParameter(java.lang.Object, java.lang.String)
#191:remove;0:remove: -1: Return v49
block 37, pred-counts: 1, succ-count: 0, filled: true, sealed: false
predecessors: 33
successors: -
no phis
#191:remove;0:remove: -1: Return v54
After dead code removal a single return is left with the non null assumption:
block 34, pred-counts: 1, succ-count: 1, filled: true, sealed: false
predecessors: 32
successors: 35 (no try/catch successors)
no phis
#187:remove;0:remove: -1: Assume v49 <- v54; not null
#188:remove;0:remove: -1: Goto block 35
block 35, pred-counts: 1, succ-count: 0, filled: true, sealed: false
predecessors: 34
successors: -
no phis
#191:remove;0:remove: -1: Return v49
For reference the source of the method is:
public fun remove(key: K): V? {
val previous: V?
lock.synchronized {
previous = map.remove(key)
if (previous != null) {
size -= safeSizeOf(key, previous)
}
}
if (previous != null) {
entryRemoved(false, key, previous, null)
}
return previous
}
with the default implementation of entryRemoved doing nothing:
protected open fun entryRemoved(evicted: Boolean, key: K, oldValue: V, newValue: V?) {}
sg...@google.com
The underlying issue is that branch simplification (as part of dead code removal) is comparing return values through their aliased values (in valuesAreIdentical) potentially conflating return values with different assume instructions. As this happens before collecting optimization info this can lead to the wrong dynamic type for the return value.
private boolean valuesAreIdentical(Value value, Value other) {
value = value.getAliasedValue();
other = other.getAliasedValue();
if (value == other) {
return true;
}
if (value.isPhi() || other.isPhi()) {
return false;
}
return instructionsDefineIdenticalValues(value.definition, other.definition);
}
ap...@google.com
Project: r8
Branch: main
Author: Søren Gjesse <
Link:
Add reproduction of
Expand for full commit details
Add reproduction of b/395489597
Bug: b/395489597
Change-Id: If769fae5db93ee8e2cfc1cf9feeddd847b00832b
Files:
- A
src/test/java/com/android/tools/r8/deadcode/DifferentAssumptionsOnReturnValuesTest.java
Hash: 92f532e6521e5e18a35f365824ad402acb325b3e
Date: Wed Feb 12 12:02:20 2025
ap...@google.com
Project: r8
Branch: main
Author: Søren Gjesse <
Link:
Don't use aliased values when comparing values for basic block subsumption
Expand for full commit details
Don't use aliased values when comparing values for basic block subsumption
Aliased values can be defined by semantically different assume instructions and then the resulting value chosen can have
information which does not correctly represent both values.
Fixes: b/395489597
Change-Id: Ieaeb4fbf8d49cdc60d2912791cc2b701d42d2b34
Files:
- M
src/main/java/com/android/tools/r8/ir/analysis/equivalence/BasicBlockBehavioralSubsumption.java - M
src/test/java/com/android/tools/r8/deadcode/DifferentAssumptionsOnReturnValuesTest.java
Hash: 94489e15f0c4d1477fe0b16912f6f3bfa4957f09
Date: Wed Feb 12 13:21:29 2025
ap...@google.com
Project: r8
Branch: 8.9
Author: Søren Gjesse <
Link:
Version 8.9.27
Expand for full commit details
Version 8.9.27
R=christofferqa@google.com
Bug: b/395489597
Change-Id: I4a2483c145c93f9370e3af2c59bd3b2be487441d
Files:
- M
src/main/java/com/android/tools/r8/Version.java
Hash: 1d6cbecc9afd9991f4db5f517bdeba1da9347093
Date: Wed Feb 12 13:57:20 2025
ap...@google.com
Project: r8
Branch: 8.9
Author: Søren Gjesse <
Link:
Don't use aliased values when comparing values for basic block subsumption
Expand for full commit details
Don't use aliased values when comparing values for basic block subsumption
Aliased values can be defined by semantically different assume instructions and then the resulting value chosen can have
information which does not correctly represent both values.
R=christofferqa@google.com
Fixes: b/395489597
Change-Id: Ieaeb4fbf8d49cdc60d2912791cc2b701d42d2b34
Files:
- M
src/main/java/com/android/tools/r8/ir/analysis/equivalence/BasicBlockBehavioralSubsumption.java - M
src/test/java/com/android/tools/r8/deadcode/DifferentAssumptionsOnReturnValuesTest.java
Hash: 3f8eebe693e4d73cfbdaf5e5761733ec47d3b775
Date: Wed Feb 12 13:57:12 2025
ap...@google.com
Project: r8
Branch: 8.9
Author: Søren Gjesse <
Link:
Add reproduction of
Expand for full commit details
Add reproduction of b/395489597
R=christofferqa@google.com
Bug: b/395489597
Change-Id: If769fae5db93ee8e2cfc1cf9feeddd847b00832b
Files:
- A
src/test/java/com/android/tools/r8/deadcode/DifferentAssumptionsOnReturnValuesTest.java
Hash: 49cade9501045641cf63bca54ab15728ea95f16d
Date: Wed Feb 12 13:57:05 2025
ap...@google.com
Project: r8
Branch: 8.8
Author: Søren Gjesse <
Link:
Don't use aliased values when comparing values for basic block subsumption
Expand for full commit details
Don't use aliased values when comparing values for basic block subsumption
Aliased values can be defined by semantically different assume instructions and then the resulting value chosen can have
information which does not correctly represent both values.
R=christofferqa@google.com
Fixes: b/395489597
Change-Id: Ieaeb4fbf8d49cdc60d2912791cc2b701d42d2b34
Files:
- M
src/main/java/com/android/tools/r8/ir/analysis/equivalence/BasicBlockBehavioralSubsumption.java - M
src/test/java/com/android/tools/r8/deadcode/DifferentAssumptionsOnReturnValuesTest.java
Hash: e7d78e09612fe9f67730cddc22cf03798a1ddd38
Date: Wed Feb 12 15:02:14 2025
ap...@google.com
Project: r8
Branch: 8.8
Author: Søren Gjesse <
Link:
Add reproduction of
Expand for full commit details
Add reproduction of b/395489597
R=christofferqa@google.com
Bug: b/395489597
Change-Id: If769fae5db93ee8e2cfc1cf9feeddd847b00832b
Files:
- A
src/test/java/com/android/tools/r8/deadcode/DifferentAssumptionsOnReturnValuesTest.java
Hash: 7697806c5d5f04330289dca95a11b755ed05e691
Date: Wed Feb 12 15:01:41 2025
ap...@google.com
Project: r8
Branch: 8.8
Author: Søren Gjesse <
Link:
Version 8.8.34
Expand for full commit details
Version 8.8.34
R=christofferqa@google.com
Bug: b/395489597
Change-Id: I2475873aa690ee588f913f12d21dbf16e07c0468
Files:
- M
src/main/java/com/android/tools/r8/Version.java
Hash: e4c4b8c78485b4b879001e60600d9ef799083345
Date: Wed Feb 12 15:02:22 2025
ap...@google.com
Project: r8
Branch: 8.7
Author: Søren Gjesse <
Link:
Version 8.7.34
Expand for full commit details
Version 8.7.34
R=christofferqa@google.com
Bug: b/395489597
Change-Id: I4946bdfc811a0211119532ed2b23f737ff5668c3
Files:
- M
src/main/java/com/android/tools/r8/Version.java
Hash: 44d960a65196e49153711487c3f24793ded7c057
Date: Wed Feb 12 15:03:02 2025
ap...@google.com
Project: r8
Branch: 8.7
Author: Søren Gjesse <
Link:
Don't use aliased values when comparing values for basic block subsumption
Expand for full commit details
Don't use aliased values when comparing values for basic block subsumption
Aliased values can be defined by semantically different assume instructions and then the resulting value chosen can have
information which does not correctly represent both values.
R=christofferqa@google.com
Fixes: b/395489597
Change-Id: Ieaeb4fbf8d49cdc60d2912791cc2b701d42d2b34
Files:
- M
src/main/java/com/android/tools/r8/ir/analysis/equivalence/BasicBlockBehavioralSubsumption.java - M
src/test/java/com/android/tools/r8/deadcode/DifferentAssumptionsOnReturnValuesTest.java
Hash: 57e30e3ab2df1593c339d6fafa4b9c5162dad653
Date: Wed Feb 12 15:02:55 2025
ap...@google.com
Project: r8
Branch: 8.7
Author: Søren Gjesse <
Link:
Add reproduction of
Expand for full commit details
Add reproduction of b/395489597
R=christofferqa@google.com
Bug: b/395489597
Change-Id: If769fae5db93ee8e2cfc1cf9feeddd847b00832b
Files:
- A
src/test/java/com/android/tools/r8/deadcode/DifferentAssumptionsOnReturnValuesTest.java
Hash: ede31119cc4b72e116e3ddbf3bd6d246ee7baff8
Date: Wed Feb 12 15:02:48 2025
sg...@google.com
This fix will make it into one of the next AGP alpha versions (most likely 8.10.0-alpha06 or 8.10.0-alpha06). Until that one can use R8 version 8.10.8-dev, see
an...@google.com
Thank you for your patience while our engineering team worked to resolve this issue. A fix for this issue is now available in:
- Android Studio Meerkat | 2024.3.1 RC 2
- Android Gradle Plugin 8.9.0-rc02
We encourage you to try the latest update.
If you notice further issues or have questions, please file a new bug report.
Thank you for taking the time to submit feedback — we really appreciate it!
an...@google.com
The fixes for this issue are now also available in:
- Android Studio Ladybug Feature Drop | 2024.2.2 Patch 2
- Android Gradle Plugin 8.8.2
We encourage you to try the latest update.
If you notice further issues or have questions, please file a new bug report.
Description
When minifying a project that uses Leanback (View-based) with the version of R8 that is included with AGP 8.10.0-alpha04, I get the crash that is included below.
This only happens when
minifyEnabledis set totrueanddebuggableis set tofalse.If I use AGP 8.8.0 then the app doesn't crash.
I have attached a project that can be used to reproduce the issue (it's basically the Leanback template that's in Android Studio with some small modifications).
Crash log: