JVM bugfix for failing to running JTreg test 'hotspot/test/compiler/7116216/StackOverflow.java'

[Edward Nevill]

Hi Mingliang Yi,

Thanks for finding this.

The reason this code works on aarch64 and does not work on aarch32 is due to the differing implementations of enter and leave on aarch64 vs aarch32.

On aarch64 enter() does

  void enter()
  {
    stp(rfp, lr, Address(pre(sp, -2 * wordSize)));
    mov(rfp, sp);
  }

whereas on aarch32 enter() does

  void enter()
  {
    stmdb(sp, RegSet::of(rfp, lr).bits());
    add(rfp, sp, wordSize);
  }

Joseph: Do you recall why aarch32 adjusts the frame pointer by 4 in enter and leave like this?

So on aarch64 the line

    __ sub(sp, rfp, ((unsigned)framesize-4) << LogBytesPerInt); // prolog

works because it just moves the fp straight back to sp, whereas on aarch32 it would need to subtract 4 to account for the additional wordSize added in enter().

This line of code is in fact just trying to create space for any callee save argument, so this line could be just replaced with

    __ sub(sp, rfp, frame::arg_reg_save_area_bytes + wordSize); // prolog

(the additional wordsize here to cater for the wordSize added in enter()).

On aarch32/aarch64 frame::arg_reg_save_area_bytes is guaranteed to be 0 so this line is redundant. However if we ever decided to have callee saved arguments this would not be redundant.

I have modified your patch to leave this line of code in, but commented out with an additional assertion that arg_reg_save_area_bytes == 0.

I have also tidied up some redundant comments and fixed the enum_layout which was still based on aarch64.

Webrev here: http://cr.openjdk.java.net/~enevill/8148355/webrev

All the best,
Ed.


==================================================
Bug description:

   JTreg test fail: hotspot/test/compiler/7116216/StackOverflow.java, with
error "assert(false) failed: DEBUG MESSAGE: no r13 to peel back"

The testcase aims to test when the callee have a lot of locals, if we've
got enough room on the stack for it. In the function
InterpreterGenerator::generate_stack_overflow_check in the source file
templateInterpreter_aarch32.cpp, we can see how it is done. First check if
the frame is greater than one page in size. If not, we finish checking, if
so, then we will use max_pages * page_size to expand the stack, and check
if the sp is out of the limit stack scope. If not, we finish checking, if
so, we will go to generate_throw_exception to throw a stackoverflow
exception. But at present, it left with “__ stop("no r13 to peel back");”
before going to generate_throw_exception which will stop JVM and cause fail
“assert(false)”.

At the same time, we find a bug in the function generate_throw_exception in
the source file stubGenerator_aarch32.cpp, where we generate the
stackoverflow exception. The assembly it produces like this:

push  {fp, lr}

add  fp, sp, #4

sub  sp, fp, #0

add  r9, pc, #0

str    r9, [r8, #324]

…

“add      fp, sp, #4” makes the fp point to lr in the stack. “sub  sp, fp,
#0” makes the sp and fp point to the same place(lr). It would destroy the
stack. The sp should point to fp in the stack.



Bug solution:

   For the first problem, We should delete the “__ stop("no r13 to peel
back");”. And let it go on.

   For the second, I delete the c++ code which generated the assembly “sub
sp, fp, #0”. It will not have the stack trample problem.



Bug patch:

diff -r 06eed0568597 src/cpu/aarch32/vm/templateInterpreter_aarch32.cpp

--- a/src/cpu/aarch32/vm/templateInterpreter_aarch32.cpp        Tue Jan 05
09:17:02 2016 +0000

+++ b/src/cpu/aarch32/vm/templateInterpreter_aarch32.cpp        Mon Jan 11
11:09:16 2016 +0800

@@ -518,8 +518,8 @@

   // unnecessary because the sender SP in r13 is always aligned, but

   // it doesn't hurt.

   //__ bic(sp, r13, 7);

-  __ stop("no r13 to peel back");

   // Note: the restored frame is not necessarily interpreted.

   // Use the shared runtime version of the StackOverflowError.

   assert(StubRoutines::throw_StackOverflowError_entry() != NULL, "stub not
yet generated");



diff -r 06eed0568597 src/cpu/aarch32/vm/stubGenerator_aarch32.cpp

--- a/src/cpu/aarch32/vm/stubGenerator_aarch32.cpp      Tue Jan 05 09:17:02
2016 +0000

+++ b/src/cpu/aarch32/vm/stubGenerator_aarch32.cpp      Mon Jan 11 15:03:17
2016 +0800

@@ -1640,7 +1640,7 @@

     assert(is_even(framesize/2), "sp not 16-byte aligned");

     // lr and fp are already in place

-    __ sub(sp, rfp, ((unsigned)framesize-4) << LogBytesPerInt); // prolog

     int frame_complete = __ pc() - start;