ARM: Fix JIT bug that miscompiles Eclipse

[Andrew Dinn]

Ok, I managed to find a reproducer for this problem. You need a loop
which uses the loop counter in a shift and then uses it afterwards in
the loop body. See attached test class Test8.

With the original JVM code this test hangs. It runs ok after the patch.

The ASM listing for the two runs shows that the patch avoids the loop
var being ANDed with 31 before the loop test is retried.

So, the fix is good to go.

regards,


Andrew Dinn
-----------

On 06/06/12 15:22, Andrew Haley wrote:
> Eclipse sometimes hangs with the current ARM JIT.  It turns out that we
> are corrupting the count operand in
> 
>   a << b
> 
> If b lives in a local it gets ANDed with 31.  Most shift counts are
> less than 31 so it doesn't usually matter.  In Eclipse, though, we
> have
> 
>    for (int i = 1; i <= 32; i++) {
>        if ((sourcePriority & (1 << i)) != 0) {
>            ...
>        }
>    }
> 
> This never terminates because each time around the loop i is ANDed
> with 31.
> 
> I guess that this loop should really be
> 
>    for (int i = 0; i < 32; i++) {
> 
> i.e. this may be a bug in Eclipse.  But we shouldn't miscompile it.  I
> think this bug has been in the ARM JIT for quite a long time.
> 
> I also took the opportunity to ask the CompilerOracle which methods
> should be JIT-compiled.  This means we now have the flexibility to
> disable specific method compilation from the command line.
> 
> Andrew.
> 
> 
> # HG changeset patch
> # User aph
> # Date 1338991762 14400
> # Node ID 51380f2370a7c982f09dc1070c6c1c66218b5600
> # Parent  d1154290751107fc148173d73bc0cdef145f2230
> Fix JIT bug that miscompiles org.eclipse.ui.internal.contexts.ContextAuthority.sourceChanged
> 2012-06-06  Andrew Haley  <aph at redhat.com>
> 
>         * thumb2.cpp (Thumb2_Compile): Ask the CompilerOracle if we should
>         compile this method.
>         (Thumb2_iOp): Use a temporary to hold the shift count.
> 
> diff -r d11542907511 -r 51380f2370a7 src/cpu/zero/vm/thumb2.cpp
> --- a/src/cpu/zero/vm/thumb2.cpp        Thu May 31 06:42:18 2012 -0400
> +++ b/src/cpu/zero/vm/thumb2.cpp        Wed Jun 06 10:09:22 2012 -0400
> @@ -68,6 +68,7 @@
>  #include <ucontext.h>
>  #include "precompiled.hpp"
>  #include "interpreter/bytecodes.hpp"
> +#include "compiler/compilerOracle.hpp"
> 
>  #define opc_nop                        0x00
>  #define opc_aconst_null                0x01
> @@ -3969,8 +3970,12 @@
>    case opc_ishl:
>    case opc_ishr:
>    case opc_iushr:
> -    and_imm(jinfo->codebuf, r_rho, r_rho, 31);
> -    break;
> +    {
> +      unsigned tmp_reg = Thumb2_Tmp(jinfo, 1 << r_lho | 1 << r_rho | 1 << r);
> +      and_imm(jinfo->codebuf, tmp_reg, r_rho, 31);
> +      r_rho = tmp_reg;
> +      break;
> +    }
>    }
>    dop_reg(jinfo->codebuf, dOps[opc-opc_iadd], r, r_lho, r_rho, 0, 0);
>  }
> @@ -7044,10 +7049,15 @@
>    if (!(CPUInfo & ARCH_THUMB2))
>         UseCompiler = false;
> 
> -  if (!UseCompiler || method->is_not_compilable()) {
> -       ic->set(ic->state(), 1);
> -       bc->set(ic->state(), 1);
> -       return 0;
> +  {
> +    bool ignore;
> +    methodHandle mh(thread, method);
> +    if (!UseCompiler || method->is_not_compilable()
> +       || CompilerOracle::should_exclude(mh, ignore)) {
> +      ic->set(ic->state(), 1);
> +      bc->set(ic->state(), 1);
> +      return 0;
> +    }
>    }
> 
>    slow_entry = *(unsigned *)method->from_interpreted_entry();
> 
> 
> 
> 
> 

-- 
regards,


Andrew Dinn
-----------
Principal Software Engineer
Red Hat UK Ltd
Registered in UK and Wales under Company Registration No. 3798903
Directors: Michael Cunningham (USA), Mark Hegarty (Ireland), Matt Parson
(USA), Charlie Peters (USA)
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: asm-listings.txt
Url: http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20120607/543b7e1d/asm-listings.txt 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Test8.java
Type: text/x-java
Size: 371 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20120607/543b7e1d/Test8.java