ARM: Fix strange crash while running JCK
Xerxes Rånby
xerxes at zafena.se
Mon Jan 16 05:27:55 PST 2012
OK comments inline.
I am in favor for this to be pushed to icedtea6 HEAD and icedtea6-1.11 release branch.
2012-01-13 16:23, Andrew Haley skrev:
> Pavel reported a strange crash when running the JCK: a segfault
> happens after the program has been running a long time.
>
> It turns out that there are two bytecode dispatch tables. One of
> these is used for normal dispatch, one of these when there is a
> safepoint. One of these is generated automagically, and one isn't.
> If you change bytecode numbering, you also have to change the
> manually-generated version of the bytecode table. I've added comments
> to this effect.
>
> The bytecodes were renumbered (at HS19, I think) but the
> manually-generated version of the bytecode table wasn't. The bytecode
> that was affected was return_register_finalizer, which is only used in
> the method java.lang.Object.<init>(). So, this crash was very rare:
> only when moving to a safepoint and executing Object.<init>(), and
> only in the interpreter: the JIT wasn't affected.
>
> I've also taken the opportunity to add bytecode numbers as comments to
> all the bytecode tables.
>
> Finally, I found another bug in the JIT: if we move to a safepoint we
> should process it before unlocking the monitor at the exit of a
> synchronized routine.
>
> Andrew.
>
>
>
> 2012-01-13 Andrew Haley <aph at redhat.com>
>
> * arm_port/hotspot/tools/mkbc.c (writeouttable): Add numbering
> comments to bytecode output.
> * arm_port/hotspot/src/cpu/zero/vm/thumb2.cpp (Thumb2_Return):
> Move safepoint before monitor is unlocked.
> * arm_port/hotspot/src/cpu/zero/vm/cppInterpreter_arm.S
> (safe_dispatch_table): Add comment.
> Correct order of do_return_register_finalizer.
> Add numbering comments.
> * arm_port/hotspot/src/cpu/zero/vm/cppInterpreter_arm.S (TRACE):
> New macro.
>
> diff -r 79014132e844 -r fc7a57de5179 arm_port/hotspot/src/cpu/zero/vm/bytecodes_arm.def
> --- a/arm_port/hotspot/src/cpu/zero/vm/bytecodes_arm.def Tue Jan 10 16:47:52 2012 +0000
> +++ b/arm_port/hotspot/src/cpu/zero/vm/bytecodes_arm.def Fri Jan 13 10:07:43 2012 -0500
> @@ -29,6 +29,10 @@
> #define FAST_BYTECODES
> #endif
>
> +/* WARNING: If you change any of these bytecodes, you must also
> + change the safe_dispatch_table in cppInterpreter_arm.S to make it
> + match. */
> +
> nop = 0x00, 1
> aconst_null = 0x01, 1
> iconst_m1 = 0x02, 1
ok
> diff -r 79014132e844 -r fc7a57de5179 arm_port/hotspot/src/cpu/zero/vm/cppInterpreter_arm.S
> --- a/arm_port/hotspot/src/cpu/zero/vm/cppInterpreter_arm.S Tue Jan 10 16:47:52 2012 +0000
> +++ b/arm_port/hotspot/src/cpu/zero/vm/cppInterpreter_arm.S Fri Jan 13 10:07:43 2012 -0500
> @@ -483,6 +483,19 @@
> .set dispatch_state, dispatch_state + 1
> .endm
>
> + @ This macro calls a user-supplied my_trace routine. It
> + @ passes the current JPC as argument zero. It can be safely
> + @ inserted at any point in the interpreter.
> + .macro TRACE
> + stmfd sp!, {r0, r1, r2, r3, r4, lr}
> + mrs r4, cpsr
> + mov r0, jpc
> + ldr r2, =my_trace
> + blx r2
> + msr cpsr, r4
> + ldmfd sp!, {r0, r1, r2, r3, r4, lr}
> + .endm
> +
> .macro DISPATCH_FINISH
> .if dispatch_state == 0
> .error "DISPATCH_FINISH without a DISPATCH_START or DISPATCH_STATE"
Nice debugging add-on.
The only thing missing now are a "How to debug the ARM assembler interpreter and Thumb2 JIT hands on tutorial":)
And possibly a my_trace function implementation example.
> @@ -4753,262 +4766,266 @@
>
> #ifdef NOTICE_SAFEPOINTS
> safe_dispatch_table:
> - .word do_nop
...
> - .word do_fast_iload_iload_N
> - .word do_return_register_finalizer
> - .word do_undefined
> - .word do_iload_0_iconst_N
...
> - .word do_fast_iload_N_iload_N
> - .word do_undefined
> - .word do_undefined
> +
> +/* WARNING: If you change any of these bytecodes, you must also
> + change the table in bytecodes_arm.def to make it match. */
> +
> + .word do_nop @ 0 0x00
...
> + .word do_fast_iload_iload_N @ 228 0xe4
> + .word do_undefined @ 229 0xe5
> + .word do_undefined @ 230 0xe6
> + .word do_return_register_finalizer @ 231 0xe7
> + .word do_undefined @ 232 0xe8
> + .word do_iload_0_iconst_N @ 233 0xe9
> + .word do_iload_0_iconst_N @ 234 0xea
...
> + .word do_fast_iload_N_iload_N @ 255 0xff
> #endif
Yes, this fix are correct, thank you.
For reference, gbenson fixed the zero cpp interpreter similarly (PR696) for compatiblity with hs20 and later.
Btw, If we decide to use shark in combination with the ARM assembler interpreter in the future then we need to re-enable
the last block of this changeset.
pr696: http://icedtea.classpath.org/hg/icedtea6/rev/f418ea6127b6
>
> SUB_DISPATCH_TABLES
> diff -r 79014132e844 -r fc7a57de5179 arm_port/hotspot/src/cpu/zero/vm/thumb2.cpp
> --- a/arm_port/hotspot/src/cpu/zero/vm/thumb2.cpp Tue Jan 10 16:47:52 2012 +0000
> +++ b/arm_port/hotspot/src/cpu/zero/vm/thumb2.cpp Fri Jan 13 10:07:43 2012 -0500
> @@ -4404,6 +4404,8 @@
>
> void Thumb2_Return(Thumb2_Info *jinfo, unsigned opcode, int bci)
> {
> + Thumb2_Safepoint(jinfo, 0, bci);
> +
> Reg r_lo, r;
> Thumb2_Stack *jstack = jinfo->jstack;
>
> @@ -4470,8 +4472,6 @@
> cbz_patch(jinfo->codebuf, ARM_R3, loc_success2);
> }
>
> - Thumb2_Safepoint(jinfo, 0, bci);
> -
> if (opcode != opc_return) {
> if (opcode == opc_lreturn || opcode == opc_dreturn) {
> Thumb2_Fill(jinfo, 2);
ok
> diff -r 79014132e844 -r fc7a57de5179 arm_port/hotspot/tools/mkbc.c
> --- a/arm_port/hotspot/tools/mkbc.c Tue Jan 10 16:47:52 2012 +0000
> +++ b/arm_port/hotspot/tools/mkbc.c Fri Jan 13 10:07:43 2012 -0500
> @@ -399,9 +399,9 @@
> fprintf(bci_f, "+%d\n", len);
> } else {
> if (table[i].impl_name)
> - fprintf(bci_f, "\t.word\t%s%s\n", prefix, table[i].impl_name);
> + fprintf(bci_f, "\t.word\t%s%s \t@ %d 0x%02x\n", prefix, table[i].impl_name, i, i);
> else
> - fprintf(bci_f, "\t.word\t%s%s\n", prefix, table[i].def_name);
> + fprintf(bci_f, "\t.word\t%s%s \t@ %d 0x%02x\n", prefix, table[i].def_name, i, i);
> }
> }
> if (depth == 0) {
ok
> @@ -416,7 +416,7 @@
> fputc('_', bci_f);
> fputs(bytecodes[table_indices[j]].name, bci_f);
> }
> - fputs(":\n", bci_f);
> + fprintf(bci_f, ":\t@ %d 0x%02x\n", i, i);
> remove_duplicates(table, i, table_indices, depth);
> writeouttable(table[i].subtable, table_indices, depth+1);
> }
ok
Cheers
Xerxes
More information about the distro-pkg-dev
mailing list