ARM: Fix broken safepoint code

Andrew Haley aph at redhat.com
Wed Jan 25 05:16:19 PST 2012


We've been seeing mysterious crashes in Eclipse.  It turns out that
ecj generates different code from javac for things like

            while (tmp != null)
                tmp = tmp.poo;

javac generates:

  8: aload_1
  9: ifnull 20
 12: aload_1
 13: getfield <Field t$Poo.poo t$Poo>
 16: astore_1
 17: goto 8

ecj generates:

 11: aload_1
 12: getfield <Field t$Poo.poo t$Poo>
 15: astore_1
 16: aload_1
 17: ifnonnull 11

And this reveals a bug in safepoint handling: conditional backwards
jumps were popping arguments from the stack before calling the
safepoint handler.  This is wrong because the garbage collector
may expect to find an object reference on the stack.

Fixed thusly.

Andrew.


2012-01-25  Andrew Haley  <aph at redhat.com>

	* openjdk-ecj/hotspot/src/cpu/zero/vm/thumb2.cpp (Thumb2_Branch):
	Remove safepoint code.
	(Thumb2_Cond_Safepoint): New function.
	(Thumb2_codegen): Call Thumb2_Cond_Safepoint() from two places.

diff -r 6179ebaffe6b arm_port/hotspot/src/cpu/zero/vm/thumb2.cpp
--- a/arm_port/hotspot/src/cpu/zero/vm/thumb2.cpp	Mon Jan 23 16:43:01 2012 +0000
+++ b/arm_port/hotspot/src/cpu/zero/vm/thumb2.cpp	Wed Jan 25 07:59:48 2012 -0500
@@ -4365,7 +4365,17 @@
   }
 }

-int Thumb2_Branch(Thumb2_Info *jinfo, unsigned bci, unsigned cond, int stackdepth)
+// If this is a backward branch, compile a safepoint check
+void Thumb2_Cond_Safepoint(Thumb2_Info *jinfo, int stackdepth, int bci) {
+  int offset = GET_JAVA_S2(jinfo->code_base + bci + 1);
+  unsigned dest_taken = bci + offset;
+
+  if (jinfo->bc_stackinfo[dest_taken] & BC_COMPILED) {
+    Thumb2_Safepoint(jinfo, stackdepth, bci);
+  }
+}
+
+int Thumb2_Branch(Thumb2_Info *jinfo, unsigned bci, unsigned cond)
 {
     int offset = GET_JAVA_S2(jinfo->code_base + bci + 1);
     unsigned dest_taken = bci + offset;
@@ -4373,10 +4383,7 @@
     unsigned loc;

     if (jinfo->bc_stackinfo[dest_taken] & BC_COMPILED) {
-      loc = forward_16(jinfo->codebuf);
-      Thumb2_Safepoint(jinfo, stackdepth, bci);
-      branch_uncond(jinfo->codebuf, jinfo->bc_stackinfo[dest_taken] & ~BC_FLAGS_MASK);
-      bcc_patch(jinfo->codebuf, NEG_COND(cond), loc);
+      branch(jinfo->codebuf, cond, jinfo->bc_stackinfo[dest_taken] & ~BC_FLAGS_MASK);
       return dest_not_taken;
     }
     loc = forward_32(jinfo->codebuf);
@@ -6323,12 +6330,13 @@
       case opc_ifnonnull: {
 	Reg r;
 	unsigned cond = opcode - opc_ifeq;
+	Thumb2_Cond_Safepoint(jinfo, stackdepth, bci);
 	if (opcode >= opc_ifnull) cond = opcode - opc_ifnull;
 	Thumb2_Fill(jinfo, 1);
 	r = POP(jstack);
 	Thumb2_Flush(jinfo);
 	cmp_imm(jinfo->codebuf, r, 0);
-	bci = Thumb2_Branch(jinfo, bci, cond, stackdepth-1);
+	bci = Thumb2_Branch(jinfo, bci, cond);
 	len = 0;
 	break;
       }
@@ -6343,13 +6351,14 @@
       case opc_if_acmpne: {
 	Reg r_lho, r_rho;
 	unsigned cond = opcode - opc_if_icmpeq;
+	Thumb2_Cond_Safepoint(jinfo, stackdepth, bci);
 	if (opcode >= opc_if_acmpeq) cond = opcode - opc_if_acmpeq;
 	Thumb2_Fill(jinfo, 2);
 	r_rho = POP(jstack);
 	r_lho = POP(jstack);
 	Thumb2_Flush(jinfo);
 	cmp_reg(jinfo->codebuf, r_lho, r_rho);
-	bci = Thumb2_Branch(jinfo, bci, cond, stackdepth-2);
+	bci = Thumb2_Branch(jinfo, bci, cond);
 	len = 0;
 	break;
       }



More information about the distro-pkg-dev mailing list