JDK-6998985 : faulty generic arraycopy on windows x86_64: 4th arg overwritten with oop
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: hs20
  • Priority: P3
  • Status: Closed
  • Resolution: Fixed
  • OS: windows_2003
  • CPU: x86
  • Submitted: 2010-11-10
  • Updated: 2011-03-08
  • Resolved: 2011-03-08
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 6 JDK 7 Other
6u25Fixed 7Fixed hs20Fixed
Description
The is a problem concerning generic arraycopy on windows x86_64. On windows, the stub gets its 4th  argument in register r9. The register gets overwritten by an oop.  In most cases, the subsequent range check will fail and the copy will be done  over the slow path. However, depending on the oops address, the range check might succeed and the copy lead to wrong results.

Comments
EVALUATION http://hg.openjdk.java.net/jdk7/build/hotspot/rev/bbefa3ca1543
25-12-2010

EVALUATION http://hg.openjdk.java.net/jdk7/hotspot/hotspot/rev/bbefa3ca1543
16-12-2010

EVALUATION The suggested fix revealed a pre-existing bug in generic_arraycopy. When the arrays to be copied are object arrays with a different element class, generic_arraycopy dispatches to checkcast_copy_entry to do the copying. Since checkcast_copy is a normal method to be called from JIT code, generic_arraycopy needs to set up the arguments for checkcast_copy. On Win64 the 5th argument is passed on the stack, that is the destination array element class for checkcast_copy and the element count for generic_arraycopy which is an int. generic_arraycopy stores the destination array element class into the stack slot of the 5th argument overwriting the integer argument with a class pointer and that results in IndexOutOfBoundsExceptions. The fix is to change the checkcast_copy_entry point and setup the argument registers before dispatching to that entry.
03-12-2010

EVALUATION http://hg.openjdk.java.net/jdk7/hotspot-comp/hotspot/rev/bbefa3ca1543
03-12-2010

EVALUATION The provided fix is correct, although I would suggest a slightly different one: http://cr.openjdk.java.net/~twisti/6998985/webrev.01/
11-11-2010

SUGGESTED FIX Should it actually be r11? +#ifdef _WIN64 + const Register r11_dst_klass = r11; // dest array klass +#else
10-11-2010

SUGGESTED FIX SAP provided the fix below with a letter of designation. -------- Original Message -------- Received: from acsinet15.oracle.com (/141.146.126.227) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Wed, 10 Nov 2010 03:18:03 -0800 Received: from rcsinet12.oracle.com (rcsinet12.oracle.com [148.87.113.124]) by acsinet15.oracle.com (Switch-3.4.2/Switch-3.4.1) with ESMTP id oAABI2i6006223 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 10 Nov 2010 11:18:02 GMT Received: from smtpgw.sap-ag.de (smtpgw02.sap-ag.de [155.56.66.97]) by rcsinet12.oracle.com (Switch-3.4.2/Switch-3.4.2) with ESMTP id oAABHw9p023680 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=FAIL); Wed, 10 Nov 2010 11:18:01 GMT From: Wintergerst, Michael <###@###.###> To: ###@###.### <###@###.###>, ###@###.### <###@###.###> CC: Siebenborn, Axel <###@###.###> Date: Wed, 10 Nov 2010 12:17:56 +0100 Subject: Bug Description + Patch Thread-Topic: Bug Description + Patch Thread-Index: AcuAyOy9MveDTLGWQo2ZDEiDIaw1oQ== Message-ID: <###@###.###> Accept-Language: en-US, de-DE Content-Language: en-US X-MS-Has-Attach: yes X-MS-TNEF-Correlator: acceptlanguage: en-US, de-DE Content-Type: multipart/mixed; boundary="_005_57FB0D12D0E5F3478D329465E18D05651508F843ADDEWDFECCR09wd_" MIME-Version: 1.0 X-Source-IP: smtpgw02.sap-ag.de [155.56.66.97] X-CT-RefId: str=0001.0A090206.4CDA7F69.0141:SCFSTAT10167653,ss=1,fgs=0 Hi Frank, hi Ivan, as discussed with Ivan ��� attached you find a bug description (incl. a test case) and a corresponding patch. Best regards, Michael ---------------------------- SAP would like to designate the patch described below as an "Error Correction" under the terms of the SCSL and contribute it for putback and integration into the Sun Java SE code base. We found a problem concerning generic arraycopy on windows x86_64. On windows, the stub gets its 4th argument in register r9. The register gets overwritten by an oop. In most cases, the subsequent range check will fail and the copy will be done over the slow path. However, depending on the oops address, the range check might succeed and the copy lead to wrong results. The included patch (towards /jdk7/hotspot-comp/hotspot/) uses r11 instead of r9 and reloads r11 later on. In addition to that, a test case is provided which reproduces the described bug. The test is run by: ���java ���Xmx4g testGenericArrayCopy���. Java: java version "1.6.0_22" Java(TM) SE Runtime Environment (build 1.6.0_22-b04) Java HotSpot(TM) 64-Bit Server VM (build 17.1-b03, mixed mode) OS: Microsoft Windows Server 2003 Enterprise x64 Edition Service Pack 2 CPU: AMD Opteron(tm) Processor 270 2.00 Ghz, 8,83 GB of RAM ----Start of the patch changeset: 1809:7e24f58fee25 tag: tip user: Axel Siebenborn <###@###.###> date: Mon Nov 08 11:29:51 2010 +0100 files: src/cpu/x86/vm/stubGenerator_x86_64.cpp description: in generic_array_copy on windows register r9 contains the 4th argument (dst_pos). The register gets overwitten by an oop. Changed r9 to r11 for _WIN64 and reload r11 when needed. diff -r e4fcbeb5a698 -r 7e24f58fee25 src/cpu/x86/vm/stubGenerator_x86_64.cpp --- a/src/cpu/x86/vm/stubGenerator_x86_64.cpp Sat Nov 06 20:35:36 2010 -0700 +++ b/src/cpu/x86/vm/stubGenerator_x86_64.cpp Mon Nov 08 11:29:51 2010 +0100 @@ -2488,7 +2488,11 @@ // registers used as temp const Register r11_length = r11; // elements count to copy const Register r10_src_klass = r10; // array klass +#ifdef _WIN64 + const Register r11_dst_klass = r9; // dest array klass +#else const Register r9_dst_klass = r9; // dest array klass +#endif // if (length < 0) return -1; __ movl(r11_length, C_RARG4); // length (elements count, 32-bits value) @@ -2505,8 +2509,13 @@ __ bind(L1); __ stop("broken null klass"); __ bind(L2); +#ifdef _WIN64 + __ load_klass(r11_dst_klass, dst); + __ cmpq(r11_dst_klass, 0); +#else __ load_klass(r9_dst_klass, dst); __ cmpq(r9_dst_klass, 0); +#endif __ jcc(Assembler::equal, L1); // this would be broken also BLOCK_COMMENT("assert done"); } @@ -2533,8 +2542,13 @@ __ jcc(Assembler::equal, L_objArray); // if (src->klass() != dst->klass()) return -1; +#ifdef _WIN64 + __ load_klass(r11_dst_klass, dst); + __ cmpq(r10_src_klass, r11_dst_klass); +#else __ load_klass(r9_dst_klass, dst); __ cmpq(r10_src_klass, r9_dst_klass); +#endif __ jcc(Assembler::notEqual, L_failed); // if (!src->is_Array()) return -1; @@ -2551,6 +2565,10 @@ } #endif +#ifdef _WIN64 + // reload length + __ movl(r11_length, C_RARG4); // length (elements count, 32-bits value) +#endif arraycopy_range_checks(src, src_pos, dst, dst_pos, r11_length, r10, L_failed); @@ -2623,10 +2641,21 @@ Label L_plain_copy, L_checkcast_copy; // test array classes for subtyping +#ifdef _WIN64 + __ load_klass(r11_dst_klass, dst); + __ cmpq(r10_src_klass, r11_dst_klass); // usual case is exact equality +#else __ load_klass(r9_dst_klass, dst); __ cmpq(r10_src_klass, r9_dst_klass); // usual case is exact equality +#endif + + __ jcc(Assembler::notEqual, L_checkcast_copy); +#ifdef _WIN64 + // reload length + __ movl(r11_length, C_RARG4); // length (elements count, 32-bits value) +#endif // Identically typed arrays can be copied without element-wise checks. arraycopy_range_checks(src, src_pos, dst, dst_pos, r11_length, r10, L_failed); -- End of the patch
10-11-2010