D8 in debug mode fails for valid local information spanning only a 'nop' [147865212]

Fixed

Bug

Status Update

No update yet.

Description

ag...@google.com

created issue #1

Jan 17, 2020 11:00AM

For the below class file (generated with the new upcoming kotlinc backend), we fail with: `Unexpected values live at entry to first block: [v2(e)]`. So, for some reason we end up not having the right liveness for the exception variable.

```
public static final java.lang.String box();
descriptor: ()Ljava/lang/String;
flags: ACC_PUBLIC, ACC_STATIC, ACC_FINAL
Code:
stack=1, locals=1, args_size=0
0: nop
1: ldc #11 // String A
3: areturn
4: astore_0
5: nop
6: ldc #13 // String B
8: areturn
Exception table:
from to target type
0 4 4 Class java/lang/IllegalStateException
StackMapTable: number_of_entries = 1
frame_type = 68 /* same_locals_1_stack_item */
stack = [ class java/lang/IllegalStateException ]
LineNumberTable:
line 2: 0
line 3: 1
line 4: 4
line 5: 6
line 6: 6
LocalVariableTable:
Start Length Slot Name Signature
5 1 0 e Ljava/lang/IllegalStateException;
RuntimeInvisibleAnnotations:
0: #7()
```

Comments

ap...@google.com <ap...@google.com> #2Jan 20, 2020 09:09AM

Project: r8
Branch: master

commit a3ad94edb255d2b7cb525a0b63e6ccab3b2dee5b
Author: Mads Ager <ager@google.com>
Date: Mon Jan 20 09:18:00 2020

Allow the local variable table to contain unobservable info.

In particular allow a local that spans only a nop instruction
and for which there is no line number on which the debugger
can stop and observe it. In that case, remove the local
from the output as it is unobservable in any case.

BUG=147865212
R=zerny@google.com

Change-Id: Iee8d37aae0133c7942e6e31aa96ed79f8bb54394

M src/main/java/com/android/tools/r8/ir/optimize/CodeRewriter.java
A src/test/java/com/android/tools/r8/regress/b147865212/FlafDump.java
A src/test/java/com/android/tools/r8/regress/b147865212/Regress147865212.java

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

ag...@google.com <ag...@google.com> #3Jan 20, 2020 10:44AM

Seems like that change fixed most of the issues. However, there are still issues with a class file generated with the kotlinc IR backend for locals which cannot be observed in the debugger.

Kotlin source:

```
fun box(): String {
try {
return "OK"
} finally {
if (1 == 1) {
val z = 2
}
if (3 == 3) {
val z = 4
}
}
}
```

Generated method in class file:

```
public static final java.lang.String box();
descriptor: ()Ljava/lang/String;
flags: ACC_PUBLIC, ACC_STATIC, ACC_FINAL
Code:
stack=1, locals=2, args_size=0
0: nop
1: ldc #9 // String OK
3: astore_0
4: nop
5: iconst_2
6: istore_1
7: nop
8: nop
9: iconst_4
10: istore_1
11: nop
12: aload_0
13: areturn
14: astore_0
15: nop
16: iconst_2
17: istore_1
18: nop
19: nop
20: iconst_4
21: istore_1
22: nop
23: aload_0
24: athrow
Exception table:
from to target type
0 4 14 any
14 15 14 any
StackMapTable: number_of_entries = 1
frame_type = 78 /* same_locals_1_stack_item */
stack = [ class java/lang/Throwable ]
LineNumberTable:
line 2: 0
line 3: 1
line 5: 4
line 6: 5
line 8: 8
line 9: 9
line 3: 13
line 11: 14
line 5: 15
line 6: 16
line 8: 19
line 9: 20
LocalVariableTable:
Start Length Slot Name Signature
7 1 1 z I
11 1 1 z I
18 1 1 z I
22 1 1 z I
RuntimeInvisibleAnnotations:
0: #7()
```

D8 complaints undefined values used during register allocation. Investigating.

ag...@google.com <ag...@google.com> #4Jan 20, 2020 10:57AM

Same issue, the unobservable local doesn't have to be introduced by a DebugLocalWrite instruction. It can be introduced by any instruction and be ended immediately. The rewriter probably just have to do this for all instructions that define values.

ap...@google.com <ap...@google.com> #5Jan 21, 2020 06:33AM

Project: r8
Branch: master

commit e5025e723f8af517279dc1b75023ce22b686b372
Author: Mads Ager <ager@google.com>
Date: Tue Jan 21 07:30:44 2020

Make sure to have an IR instruction materialized for local ends.

nop and move instructions did not materialize in the IR even if
the instructions end the live range of a local. This caused the
IR to sometimes contain an instruction that simultaneously starts
and ends a local. This complicates the rest of the pipeline.

With this change, all instructions that end the live range of
a local materialize either as an actual instruction or as
a DebugLocalRead.

BUG=147865212
R=zerny@google.com

Change-Id: Iedd0190092d6114726ece7fda137fec7bb14e54d

M src/main/java/com/android/tools/r8/cf/code/CfNop.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/b147865212/Flaf2Dump.java
M src/test/java/com/android/tools/r8/regress/b147865212/FlafDump.java
M src/test/java/com/android/tools/r8/regress/b147865212/Regress147865212.java

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

ap...@google.com <ap...@google.com> #6Jan 21, 2020 10:01AM

Project: r8
Branch: d8-1.5

commit 28fb1b2dfea92c0c0e5924b89c89c4c941cabf2b
Author: Mads Ager <ager@google.com>
Date: Tue Jan 21 09:00:19 2020

Version 1.5.70.

Cherry pick: Allow the local variable table to contain unobservable info.
CL:

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

Cherry pick: Make sure to have an IR instruction materialized for local ends.
CL:

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

BUG=147865212
R=zerny@google.com

Change-Id: I43e77c5a7dd485f6e27203a2f685435666718ac5

M src/main/java/com/android/tools/r8/Version.java
M src/main/java/com/android/tools/r8/cf/code/CfNop.java
M src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
M src/main/java/com/android/tools/r8/ir/conversion/JarSourceCode.java
A src/test/java/com/android/tools/r8/regress/b147865212/Flaf2Dump.java
A src/test/java/com/android/tools/r8/regress/b147865212/FlafDump.java
A src/test/java/com/android/tools/r8/regress/b147865212/Regress147865212.java

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

ap...@google.com <ap...@google.com> #7Jan 21, 2020 10:02AM

Project: r8
Branch: 2.0

commit aff3bcc2f9cec757e94deed40d55be841133ba00
Author: Mads Ager <ager@google.com>
Date: Tue Jan 21 10:18:42 2020

Version 2.0.20.

Cherry pick: Allow the local variable table to contain unobservable info.
CL:

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

Cherry pick: Make sure to have an IR instruction materialized for local ends.
CL:

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

BUG=147865212
R=zerny@google.com

Change-Id: I74f2fc6ce099b3c5b0b81fac44219ea9d1b7f7e4

M src/main/java/com/android/tools/r8/Version.java
M src/main/java/com/android/tools/r8/cf/code/CfNop.java
M src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
A src/test/java/com/android/tools/r8/regress/b147865212/Flaf2Dump.java
A src/test/java/com/android/tools/r8/regress/b147865212/FlafDump.java
A src/test/java/com/android/tools/r8/regress/b147865212/Regress147865212.java

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

ap...@google.com <ap...@google.com> #8Jan 21, 2020 10:02AM

Project: r8
Branch: 1.6

commit ffd5720c6a276926a1ceabef51d7d57a1a2733c2
Author: Mads Ager <ager@google.com>
Date: Tue Jan 21 10:04:29 2020

Version 1.6.61.

Cherry pick: Allow the local variable table to contain unobservable info.
CL:

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

Cherry pick: Make sure to have an IR instruction materialized for local ends.
CL:

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

BUG=147865212
R=zerny@google.com

Change-Id: I7d8e0a2ad4ba8456641d4d6650bf5eb9e41f198e

M src/main/java/com/android/tools/r8/Version.java
M src/main/java/com/android/tools/r8/cf/code/CfNop.java
M src/main/java/com/android/tools/r8/ir/conversion/IRBuilder.java
A src/test/java/com/android/tools/r8/regress/b147865212/Flaf2Dump.java
A src/test/java/com/android/tools/r8/regress/b147865212/FlafDump.java
A src/test/java/com/android/tools/r8/regress/b147865212/Regress147865212.java

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

ag...@google.com <ag...@google.com> Jan 22, 2020 10:16AM

Marked as fixed.

Issue 147865212

Description

Issue summary

Comments

ap...@google.com <ap...@google.com> #2Jan 20, 2020 09:09AM

ag...@google.com <ag...@google.com> #3Jan 20, 2020 10:44AM

ag...@google.com <ag...@google.com> #4Jan 20, 2020 10:57AM

ap...@google.com <ap...@google.com> #5Jan 21, 2020 06:33AM

ap...@google.com <ap...@google.com> #6Jan 21, 2020 10:01AM

ap...@google.com <ap...@google.com> #7Jan 21, 2020 10:02AM

ap...@google.com <ap...@google.com> #8Jan 21, 2020 10:02AM

ag...@google.com <ag...@google.com> Jan 22, 2020 10:16AM

Add comment

Issue metadata