8010927: Kitchensink crashed with SIGSEGV, Problematic frame: v ~StubRoutines::checkcast_arraycopy

Summary: Changed gen_write_ref_array_post_barrier() code on x64 to pass start address and number of copied oop elements. In generate_checkcast_copy() skip post barrier code if no elements are copied.
Reviewed-by: roland

src/cpu/x86/vm/stubGenerator_x86_32.cpp

@@ -1505,20 +1505,22 @@ class StubGenerator: public StubCodeGenerator {
     // Register "count" = -1 * number of *remaining* oops, length_arg = *total* oops.
     // Emit GC store barriers for the oops we have copied (length_arg + count),
     // and report their number to the caller.
+    assert_different_registers(to, count, rax);
+    Label L_post_barrier;
     __ addl(count, length_arg);         // transfers = (length - remaining)
     __ movl2ptr(rax, count);            // save the value
-    __ notptr(rax);                     // report (-1^K) to caller
-    __ movptr(to, to_arg);              // reload
-    assert_different_registers(to, count, rax);
-    gen_write_ref_array_post_barrier(to, count);
-    __ jmpb(L_done);
+    __ notptr(rax);                     // report (-1^K) to caller (does not affect flags)
+    __ jccb(Assembler::notZero, L_post_barrier);
+    __ jmp(L_done); // K == 0, nothing was copied, skip post barrier
 
     // Come here on success only.
     __ BIND(L_do_card_marks);
+    __ xorptr(rax, rax);                // return 0 on success
     __ movl2ptr(count, length_arg);
+
+    __ BIND(L_post_barrier);
     __ movptr(to, to_arg);              // reload
     gen_write_ref_array_post_barrier(to, count);
-    __ xorptr(rax, rax);                  // return 0 on success
 
     // Common exit point (success or failure).
     __ BIND(L_done);

src/cpu/x86/vm/stubGenerator_x86_64.cpp

@@ -1217,27 +1217,28 @@ class StubGenerator: public StubCodeGenerator {
   //
   //  Input:
   //     start    - register containing starting address of destination array
-  //     end      - register containing ending address of destination array
+  //     count    - elements count
   //     scratch  - scratch register
   //
   //  The input registers are overwritten.
-  //  The ending address is inclusive.
-  void  gen_write_ref_array_post_barrier(Register start, Register end, Register scratch) {
-    assert_different_registers(start, end, scratch);
+  //
+  void  gen_write_ref_array_post_barrier(Register start, Register count, Register scratch) {
+    assert_different_registers(start, count, scratch);
     BarrierSet* bs = Universe::heap()->barrier_set();
     switch (bs->kind()) {
       case BarrierSet::G1SATBCT:
       case BarrierSet::G1SATBCTLogging:
-
         {
           __ pusha();             // push registers (overkill)
-          // must compute element count unless barrier set interface is changed (other platforms supply count)
-          assert_different_registers(start, end, scratch);
-          __ lea(scratch, Address(end, BytesPerHeapOop));
-          __ subptr(scratch, start);               // subtract start to get #bytes
-          __ shrptr(scratch, LogBytesPerHeapOop);  // convert to element count
+          if (c_rarg0 == count) { // On win64 c_rarg0 == rcx
+            assert_different_registers(c_rarg1, start);
+            __ mov(c_rarg1, count);
+            __ mov(c_rarg0, start);
+          } else {
+            assert_different_registers(c_rarg0, count);
             __ mov(c_rarg0, start);
-          __ mov(c_rarg1, scratch);
+            __ mov(c_rarg1, count);
+          }
           __ call_VM_leaf(CAST_FROM_FN_PTR(address, BarrierSet::static_write_ref_array_post), 2);
           __ popa();
         }
@@ -1249,22 +1250,16 @@ class StubGenerator: public StubCodeGenerator {
           assert(sizeof(*ct->byte_map_base) == sizeof(jbyte), "adjust this code");
 
           Label L_loop;
+          const Register end = count;
 
+          __ leaq(end, Address(start, count, TIMES_OOP, 0));  // end == start+count*oop_size
+          __ subptr(end, BytesPerHeapOop); // end - 1 to make inclusive
           __ shrptr(start, CardTableModRefBS::card_shift);
-           __ addptr(end, BytesPerHeapOop);
           __ shrptr(end,   CardTableModRefBS::card_shift);
-           __ subptr(end, start); // number of bytes to copy
+          __ subptr(end, start); // end --> cards count
 
-          intptr_t disp = (intptr_t) ct->byte_map_base;
-          if (Assembler::is_simm32(disp)) {
-            Address cardtable(noreg, noreg, Address::no_scale, disp);
-            __ lea(scratch, cardtable);
-          } else {
-            ExternalAddress cardtable((address)disp);
-            __ lea(scratch, cardtable);
-          }
-
-          const Register count = end; // 'end' register contains bytes count now
+          int64_t disp = (int64_t) ct->byte_map_base;
+          __ mov64(scratch, disp);
           __ addptr(start, scratch);
         __ BIND(L_loop);
           __ movb(Address(start, count, Address::times_1), 0);
@@ -1916,8 +1911,7 @@ class StubGenerator: public StubCodeGenerator {
 
   __ BIND(L_exit);
     if (is_oop) {
-      __ leaq(end_to, Address(saved_to, dword_count, Address::times_4, -4));
-      gen_write_ref_array_post_barrier(saved_to, end_to, rax);
+      gen_write_ref_array_post_barrier(saved_to, dword_count, rax);
     }
     restore_arg_regs();
     inc_counter_np(SharedRuntime::_jint_array_copy_ctr); // Update counter after rscratch1 is free
@@ -2012,11 +2006,9 @@ class StubGenerator: public StubCodeGenerator {
     // Copy in multi-bytes chunks
     copy_bytes_backward(from, to, qword_count, rax, L_copy_bytes, L_copy_8_bytes);
 
-   __ bind(L_exit);
+  __ BIND(L_exit);
     if (is_oop) {
-       Register end_to = rdx;
-       __ leaq(end_to, Address(to, dword_count, Address::times_4, -4));
-       gen_write_ref_array_post_barrier(to, end_to, rax);
+      gen_write_ref_array_post_barrier(to, dword_count, rax);
     }
     restore_arg_regs();
     inc_counter_np(SharedRuntime::_jint_array_copy_ctr); // Update counter after rscratch1 is free
@@ -2055,6 +2047,7 @@ class StubGenerator: public StubCodeGenerator {
     const Register end_from    = from; // source array end address
     const Register end_to      = rcx;  // destination array end address
     const Register saved_to    = to;
+    const Register saved_count = r11;
     // End pointers are inclusive, and if count is not zero they point
     // to the last unit copied:  end_to[0] := end_from[0]
 
@@ -2072,6 +2065,8 @@ class StubGenerator: public StubCodeGenerator {
                       // r9 and r10 may be used to save non-volatile registers
     // 'from', 'to' and 'qword_count' are now valid
     if (is_oop) {
+      // Save to and count for store barrier
+      __ movptr(saved_count, qword_count);
       // no registers are destroyed by this call
       gen_write_ref_array_pre_barrier(to, qword_count, dest_uninitialized);
     }
@@ -2104,7 +2099,7 @@ class StubGenerator: public StubCodeGenerator {
 
     if (is_oop) {
     __ BIND(L_exit);
-      gen_write_ref_array_post_barrier(saved_to, end_to, rax);
+      gen_write_ref_array_post_barrier(saved_to, saved_count, rax);
     }
     restore_arg_regs();
     if (is_oop) {
@@ -2187,8 +2182,7 @@ class StubGenerator: public StubCodeGenerator {
 
     if (is_oop) {
     __ BIND(L_exit);
-      __ lea(rcx, Address(to, saved_count, Address::times_8, -8));
-      gen_write_ref_array_post_barrier(to, rcx, rax);
+      gen_write_ref_array_post_barrier(to, saved_count, rax);
     }
     restore_arg_regs();
     if (is_oop) {
@@ -2375,21 +2369,21 @@ class StubGenerator: public StubCodeGenerator {
     // Register rdx = -1 * number of *remaining* oops, r14 = *total* oops.
     // Emit GC store barriers for the oops we have copied (r14 + rdx),
     // and report their number to the caller.
-    assert_different_registers(rax, r14_length, count, to, end_to, rcx);
-    __ lea(end_to, to_element_addr);
-    __ addptr(end_to, -heapOopSize);      // make an inclusive end pointer
-    gen_write_ref_array_post_barrier(to, end_to, rscratch1);
-    __ movptr(rax, r14_length);           // original oops
-    __ addptr(rax, count);                // K = (original - remaining) oops
-    __ notptr(rax);                       // report (-1^K) to caller
-    __ jmp(L_done);
+    assert_different_registers(rax, r14_length, count, to, end_to, rcx, rscratch1);
+    Label L_post_barrier;
+    __ addptr(r14_length, count);     // K = (original - remaining) oops
+    __ movptr(rax, r14_length);       // save the value
+    __ notptr(rax);                   // report (-1^K) to caller (does not affect flags)
+    __ jccb(Assembler::notZero, L_post_barrier);
+    __ jmp(L_done); // K == 0, nothing was copied, skip post barrier
 
     // Come here on success only.
     __ BIND(L_do_card_marks);
-    __ addptr(end_to, -heapOopSize);         // make an inclusive end pointer
-    gen_write_ref_array_post_barrier(to, end_to, rscratch1);
     __ xorptr(rax, rax);              // return 0 on success
 
+    __ BIND(L_post_barrier);
+    gen_write_ref_array_post_barrier(to, r14_length, rscratch1);
+
     // Common exit point (success or failure).
     __ BIND(L_done);
     __ movptr(r13, Address(rsp, saved_r13_offset * wordSize));

test/compiler/8010927/Test8010927.java

+/*
+ * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8010927
+ * @summary Kitchensink crashed with SIGSEGV, Problematic frame: v ~StubRoutines::checkcast_arraycopy
+ * @library /testlibrary/whitebox /testlibrary
+ * @build Test8010927
+ * @run main ClassFileInstaller sun.hotspot.WhiteBox
+ * @run main/othervm -XX:+UnlockDiagnosticVMOptions -XX:+IgnoreUnrecognizedVMOptions -XX:+WhiteBoxAPI -Xbootclasspath/a:. -Xmx64m -XX:NewSize=20971520 -XX:MaxNewSize=32m -XX:-UseTLAB -XX:-UseParNewGC -XX:-UseAdaptiveSizePolicy Test8010927
+ */
+
+import sun.hotspot.WhiteBox;
+import java.lang.reflect.Field;
+import sun.misc.Unsafe;
+
+/**
+ * The test creates uncommitted space between oldgen and young gen
+ * by specifying MaxNewSize bigger than NewSize.
+ * NewSize = 20971520 = (512*4K) * 10 for 4k pages
+ * Then it tries to execute arraycopy() with elements type check
+ * to the array at the end of survive space near unused space.
+ */
+
+public class Test8010927 {
+
+  private static final Unsafe U;
+
+  static {
+    try {
+      Field unsafe = Unsafe.class.getDeclaredField("theUnsafe");
+      unsafe.setAccessible(true);
+      U = (Unsafe) unsafe.get(null);
+    } catch (Exception e) {
+      throw new Error(e);
+    }
+  }
+
+  public static Object[] o;
+
+  public static final boolean debug = Boolean.getBoolean("debug");
+
+  // 2 different obect arrays but same element types
+  static Test8010927[] masterA;
+  static Object[] masterB;
+  static final Test8010927 elem = new Test8010927();
+  static final WhiteBox wb = WhiteBox.getWhiteBox();
+
+  static final int obj_header_size = U.ARRAY_OBJECT_BASE_OFFSET;
+  static final int heap_oop_size = wb.getHeapOopSize();
+  static final int card_size = 512;
+  static final int one_card = (card_size - obj_header_size)/heap_oop_size;
+
+  static final int surv_size = 2112 * 1024;
+
+  // The size is big to not fit into survive space.
+  static final Object[] cache = new Object[(surv_size / card_size)];
+
+  public static void main(String[] args) {
+    masterA = new Test8010927[one_card];
+    masterB = new Object[one_card];
+    for (int i = 0; i < one_card; ++i) {
+      masterA[i] = elem;
+      masterB[i] = elem;
+    }
+
+    // Move cache[] to the old gen.
+    long low_limit = wb.getObjectAddress(cache);
+    System.gc();
+    // Move 'cache' to oldgen.
+    long upper_limit = wb.getObjectAddress(cache);
+    if ((low_limit - upper_limit) > 0) { // substaction works with unsigned values
+      // OldGen is placed before youngger for ParallelOldGC.
+      upper_limit = low_limit + 21000000l; // +20971520
+    }
+    // Each A[one_card] size is 512 bytes,
+    // it will take about 40000 allocations to trigger GC.
+    // cache[] has 8192 elements so GC should happen
+    // each 5th iteration.
+    for(long l = 0; l < 20; l++) {
+      fill_heap();
+      if (debug) {
+        System.out.println("test oop_disjoint_arraycopy");
+      }
+      testA_arraycopy();
+      if (debug) {
+        System.out.println("test checkcast_arraycopy");
+      }
+      testB_arraycopy();
+      // Execute arraycopy to the topmost array in young gen
+      if (debug) {
+        int top_index = get_top_address(low_limit, upper_limit);
+        if (top_index >= 0) {
+          long addr = wb.getObjectAddress(cache[top_index]);
+          System.out.println("top_addr: 0x" + Long.toHexString(addr) + ", 0x" + Long.toHexString(addr + 512));
+        }
+      }
+    }
+  }
+  static void fill_heap() {
+    for (int i = 0; i < cache.length; ++i) {
+      o = new Test8010927[one_card];
+      System.arraycopy(masterA, 0, o, 0, masterA.length);
+      cache[i] = o;
+    }
+    for (long j = 0; j < 256; ++j) {
+      o = new Long[10000]; // to trigger GC
+    }
+  }
+  static void testA_arraycopy() {
+    for (int i = 0; i < cache.length; ++i) {
+      System.arraycopy(masterA, 0, cache[i], 0, masterA.length);
+    }
+  }
+  static void testB_arraycopy() {
+    for (int i = 0; i < cache.length; ++i) {
+      System.arraycopy(masterB, 0, cache[i], 0, masterB.length);
+    }
+  }
+  static int get_top_address(long min, long max) {
+    int index = -1;
+    long addr = min;
+    for (int i = 0; i < cache.length; ++i) {
+      long test = wb.getObjectAddress(cache[i]);
+      if (((test - addr) > 0) && ((max - test) > 0)) { // substaction works with unsigned values
+        addr = test;
+        index = i;
+      }
+    }
+    return index;
+  }
+}