-
Bug
-
Resolution: Fixed
-
P3
-
hs25
-
b47
-
x86
-
generic
Issue | Fix Version | Assignee | Priority | Status | Resolution | Resolved In Build |
---|---|---|---|---|---|---|
JDK-8023846 | 8 | Bharadwaj Yadavalli | P3 | Resolved | Fixed | b105 |
This was detected in Graal, as yet not reproduced in the plain compiler despite some efforts, but it is definitely a bug:
(1) [corrected email from original reporter]
The symptom is a crash when Graal compiles a call to 'java.util.zip.CRC32.update(int b)'. By stepping through the call in gdb, I see that execution ends up in the code produced by InterpreterGenerator::generate_CRC32_update_entry(). Everything seems reasonable until line 883[1]. I believe that line should be:
__ mov(rsp, r13); // set sp to sender sp
(2) [second email reporting second issue]
There's also an issue with InterpreterGenerator::generate_CRC32_updateBytes_entry() in templateInterpreter_x86_64.cpp. I've fixed it in the downstream Graal repo but it should probably be pushed upstream repo as well. The problem is that the addptr assembler instruction was being used to add a 32-bit value from the stack to a 64-bit value in a register. As far as I can tell, there is no format of AMD64 addq instruction that supports this.
The supplied patch for this bug, tested in Graal, is:
diff -r 78da293f6efa -r 5c153c59ba62 src/cpu/x86/vm/templateInterpreter_x86_64.cpp
--- a/src/cpu/x86/vm/templateInterpreter_x86_64.cpp Tue Aug 06 18:32:04 2013 +0200
+++ b/src/cpu/x86/vm/templateInterpreter_x86_64.cpp Tue Aug 06 21:40:09 2013 +0200
@@ -995,20 +995,24 @@
const Register crc = c_rarg0; // crc
const Register buf = c_rarg1; // source java byte array address
const Register len = c_rarg2; // length
+ const Register off = len; // offset (never overlaps with 'len')
// Arguments are reversed on java expression stack
- __ movl(len, Address(rsp, wordSize)); // Length
// Calculate address of start element
if (kind == Interpreter::java_util_zip_CRC32_updateByteBuffer) {
__ movptr(buf, Address(rsp, 3*wordSize)); // long buf
- __ addptr(buf, Address(rsp, 2*wordSize)); // + offset
+ __ movl(len, Address(rsp, 2*wordSize)); // offset
+ __ addq(buf, len); // + offset
__ movl(crc, Address(rsp, 5*wordSize)); // Initial CRC
} else {
__ movptr(buf, Address(rsp, 3*wordSize)); // byte[] array
__ addptr(buf, arrayOopDesc::base_offset_in_bytes(T_BYTE)); // + header size
- __ addptr(buf, Address(rsp, 2*wordSize)); // + offset
+ __ movl(len, Address(rsp, 2*wordSize)); // offset
+ __ addq(buf, len); // + offset
__ movl(crc, Address(rsp, 4*wordSize)); // Initial CRC
}
+ // Can now load 'len' since we're finished with 'off'
+ __ movl(len, Address(rsp, wordSize)); // Length
__ super_call_VM_leaf(CAST_FROM_FN_PTR(address, StubRoutines::updateBytesCRC32()), crc, buf, len);
// result in rax
(1) [corrected email from original reporter]
The symptom is a crash when Graal compiles a call to 'java.util.zip.CRC32.update(int b)'. By stepping through the call in gdb, I see that execution ends up in the code produced by InterpreterGenerator::generate_CRC32_update_entry(). Everything seems reasonable until line 883[1]. I believe that line should be:
__ mov(rsp, r13); // set sp to sender sp
(2) [second email reporting second issue]
There's also an issue with InterpreterGenerator::generate_CRC32_updateBytes_entry() in templateInterpreter_x86_64.cpp. I've fixed it in the downstream Graal repo but it should probably be pushed upstream repo as well. The problem is that the addptr assembler instruction was being used to add a 32-bit value from the stack to a 64-bit value in a register. As far as I can tell, there is no format of AMD64 addq instruction that supports this.
The supplied patch for this bug, tested in Graal, is:
diff -r 78da293f6efa -r 5c153c59ba62 src/cpu/x86/vm/templateInterpreter_x86_64.cpp
--- a/src/cpu/x86/vm/templateInterpreter_x86_64.cpp Tue Aug 06 18:32:04 2013 +0200
+++ b/src/cpu/x86/vm/templateInterpreter_x86_64.cpp Tue Aug 06 21:40:09 2013 +0200
@@ -995,20 +995,24 @@
const Register crc = c_rarg0; // crc
const Register buf = c_rarg1; // source java byte array address
const Register len = c_rarg2; // length
+ const Register off = len; // offset (never overlaps with 'len')
// Arguments are reversed on java expression stack
- __ movl(len, Address(rsp, wordSize)); // Length
// Calculate address of start element
if (kind == Interpreter::java_util_zip_CRC32_updateByteBuffer) {
__ movptr(buf, Address(rsp, 3*wordSize)); // long buf
- __ addptr(buf, Address(rsp, 2*wordSize)); // + offset
+ __ movl(len, Address(rsp, 2*wordSize)); // offset
+ __ addq(buf, len); // + offset
__ movl(crc, Address(rsp, 5*wordSize)); // Initial CRC
} else {
__ movptr(buf, Address(rsp, 3*wordSize)); // byte[] array
__ addptr(buf, arrayOopDesc::base_offset_in_bytes(T_BYTE)); // + header size
- __ addptr(buf, Address(rsp, 2*wordSize)); // + offset
+ __ movl(len, Address(rsp, 2*wordSize)); // offset
+ __ addq(buf, len); // + offset
__ movl(crc, Address(rsp, 4*wordSize)); // Initial CRC
}
+ // Can now load 'len' since we're finished with 'off'
+ __ movl(len, Address(rsp, wordSize)); // Length
__ super_call_VM_leaf(CAST_FROM_FN_PTR(address, StubRoutines::updateBytesCRC32()), crc, buf, len);
// result in rax
- backported by
-
JDK-8023846 Bad code generated for certain interpreted CRC intrinsics, 2 cases
-
- Resolved
-
- relates to
-
JDK-7088419 Use x86 Hardware CRC32 Instruction with java.util.zip.CRC32
-
- Resolved
-