- 
    Bug 
- 
    Resolution: Fixed
- 
     P3 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
 
-