RFR(s): AARCH64: 8147805: C1 segmentation fault due to inline Unsafe::getAndSetObject
Hi All, Could some one help review this AArch64 C1 issue? Issue happens when inline unsafe.getAndSet(data) in C1 and UseCompressedOops flag is true, register is compressed for store, but it is not restored into decompressed form. Later compressed result is used as reference address and goes wrong. Bug: https://bugs.openjdk.java.net/browse/JDK-8147805 webrev: http://cr.openjdk.java.net/~hshi/8147805/webrev/ Small test case in http://cr.openjdk.java.net/~hshi/8147805/TestUnsafe.java Crash can be reproduced by java -XX:TieredStopAtLevel=3 -XX:+TieredCompilation -Xms4G -Xmx4G TestUnsafe In following method, n is stored two times, first in unsafe.getAndSet, second when store old.next. public Node foo(Node n) { Node old; old = this.getAndSet(n); // inline sun.misc.Unsafe::getAndSetObject here, shift first time for store old.next = n; // n is used again and store into old.next, shift again for store return old; } In generated assemlby, can see "x2" is shifted but not restored 0x0000007f943af3dc: lsr x2, x2, #3 // x2 is shifted but not restored 0x0000007f943af3e0: add x4, x1, #0xc 0x0000007f943af3e4: ldaxr w3, [x4] 0x0000007f943af3e8: stlxr w9, w2, [x4] 0x0000007f943af3ec: cbnz w9, 0x0000007f943af3e4 0x0000007f943af3f0: lsl x3, x3, #3 0x0000007f943af3f4: dmb ish 0x0000007f943af504: lsr x8, x2, #3 // x2 is shifted again and wrong 0x0000007f943af508: str w8, [x0,#16] 0x0000007f943af50c: lsr x2, x0, #9 0x0000007f943af510: strb wzr, [x2,x1,lsl #0] ;*putfield next ; - TestUnsafe::foo@11 (line 25) Patch is using rscratch1 to hold heap_oop address for store when UseCompressedOops is true. So later use still get correct object address. Regards Hui
On 01/20/2016 01:30 PM, Hui Shi wrote:
Could some one help review this AArch64 C1 issue?
OK, thanks, I'm looking at this to make sure thus problem does not exist elsewhere. Andrew.
On Wed, 2016-01-20 at 21:30 +0800, Hui Shi wrote:
Hi All,
Could some one help review this AArch64 C1 issue? Issue happens when inline unsafe.getAndSet(data) in C1 and UseCompressedOops flag is true, register is compressed for store, but it is not restored into decompressed form. Later compressed result is used as reference address and goes wrong.
Bug: https://bugs.openjdk.java.net/browse/JDK-8147805 webrev: http://cr.openjdk.java.net/~hshi/8147805/webrev/ Small test case in http://cr.openjdk.java.net/~hshi/8147805/TestUnsafe.java Crash can be reproduced by java -XX:TieredStopAtLevel=3 -XX:+TieredCompilation -Xms4G -Xmx4G TestUnsafe
Hi Hui Shi, Thanks for finding this. Your change looks correct, but if I make suggest the following smaller change which achieves the same. diff -r 46c1abd5c34d src/cpu/aarch64/vm/c1_LIRAssembler_aarch64.cpp --- a/src/cpu/aarch64/vm/c1_LIRAssembler_aarch64.cpp Tue Jan 12 14:55:15 2016 +0000 +++ b/src/cpu/aarch64/vm/c1_LIRAssembler_aarch64.cpp Wed Jan 20 14:16:56 2016 +0000 @@ -3169,7 +3169,8 @@ Register obj = as_reg(data); Register dst = as_reg(dest); if (is_oop && UseCompressedOops) { - __ encode_heap_oop(obj); + __ encode_heap_oop(rscratch1, obj); + obj = rscratch1; } assert_different_registers(obj, addr.base(), tmp, rscratch2, dst); Label again; Regards, Ed.
On 01/20/2016 02:21 PM, Edward Nevill wrote:
On Wed, 2016-01-20 at 21:30 +0800, Hui Shi wrote:
Hi All,
Could some one help review this AArch64 C1 issue? Issue happens when inline unsafe.getAndSet(data) in C1 and UseCompressedOops flag is true, register is compressed for store, but it is not restored into decompressed form. Later compressed result is used as reference address and goes wrong.
Bug: https://bugs.openjdk.java.net/browse/JDK-8147805 webrev: http://cr.openjdk.java.net/~hshi/8147805/webrev/ Small test case in http://cr.openjdk.java.net/~hshi/8147805/TestUnsafe.java Crash can be reproduced by java -XX:TieredStopAtLevel=3 -XX:+TieredCompilation -Xms4G -Xmx4G TestUnsafe
Hi Hui Shi,
Thanks for finding this. Your change looks correct, but if I make suggest the following smaller change which achieves the same.
diff -r 46c1abd5c34d src/cpu/aarch64/vm/c1_LIRAssembler_aarch64.cpp --- a/src/cpu/aarch64/vm/c1_LIRAssembler_aarch64.cpp Tue Jan 12 14:55:15 2016 +0000 +++ b/src/cpu/aarch64/vm/c1_LIRAssembler_aarch64.cpp Wed Jan 20 14:16:56 2016 +0000 @@ -3169,7 +3169,8 @@ Register obj = as_reg(data); Register dst = as_reg(dest); if (is_oop && UseCompressedOops) { - __ encode_heap_oop(obj); + __ encode_heap_oop(rscratch1, obj); + obj = rscratch1; } assert_different_registers(obj, addr.base(), tmp, rscratch2, dst); Label again;
I agree. I have tried this and it works well. The patch is OK with this change. Andrew.
participants (3)
-
Andrew Haley
-
Edward Nevill
-
Hui Shi