United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
Bug ID: JDK-6998985 faulty generic arraycopy on windows x86_64: 4th arg overwritten with oop
JDK-6998985 : faulty generic arraycopy on windows x86_64: 4th arg overwritten with oop

Details
Type:
Bug
Submit Date:
2010-11-10
Status:
Closed
Updated Date:
2011-03-08
Project Name:
JDK
Resolved Date:
2011-03-08
Component:
hotspot
OS:
windows_2003
Sub-Component:
compiler
CPU:
x86
Priority:
P3
Resolution:
Fixed
Affected Versions:
hs20
Fixed Versions:
hs20 (b04)

Related Reports
Backport:
Backport:

Sub Tasks

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
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
                                     
2010-11-10
SUGGESTED FIX

Should it actually be r11?

+#ifdef _WIN64
+    const Register r11_dst_klass  = r11;  // dest array klass
+#else
                                     
2010-11-10
EVALUATION

The provided fix is correct, although I would suggest a slightly different one:

http://cr.openjdk.java.net/~twisti/6998985/webrev.01/
                                     
2010-11-11
EVALUATION

http://hg.openjdk.java.net/jdk7/hotspot-comp/hotspot/rev/bbefa3ca1543
                                     
2010-12-03
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.
                                     
2010-12-03
EVALUATION

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

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



Hardware and Software, Engineered to Work Together