ARM: Fix broken safepoint code

[Xerxes Rånby]

2012-01-25 14:16, Andrew Haley skrev:
> 
> 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.

I have done some testing with this patch applied.
make check-hotspot and check-langtools pass nicely.
I have not seen any crash during todays testing of the thumb2 jit and so I think its safe to say that this patch are OK for inclusion into icedtea6 hg head and 1.11 release branch.

Cheers
Xerxes

> 
> 
> 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;
>        }