[rfc][ARM] Support for safepoint check based on memory protect rather than polling

[Andrew Dinn]

On 14/05/12 18:01, Andrew Haley wrote:
> Ok.  What testing did you do?

I used two test programs which executed a long-running tight loop. The
first performed a small number of operations in the loop body ensuring
that the backwards branch at the loop end fitted into 2 bytes. The
second performed a large number of operations in the loop body ensuring
that the backwards branch at the loop end required a 4 byte instruction.
With both programs the code also included forward branches and branches
at return points so they should have exercised all the paths into and
through the code generation routine.

As a basic test of the signal handler and generated safepoint check code
I signalled the JVM using a SIGQUIT sent from a tty shell causing the
VMThread to force a safepoint synchronization. I used the debugger to
check that this forced each of the test programs to exercise the
interrupt handler and successfully redirect the test thread into the
code which calls the safepoint blocking routine.

As a stress test I sent a continuous stream of QUIT signals to the JVM
using a shell command while loop.

I did consider attempting to repeatedly force a safepoint entry from a
Java thread spawned by the test program. However, after looking through
the VM code, it seems there is only one API to do this (queuing a
VM_ForceSafepoint op to the VM_Thread -- ok two ways because you can
also queue a VM_ForceAsyncSafepoint op). There is no straightforward way
to access this API from Java -- it requires implementing a JNI native
call plus doctoring libjvm.so to export a routine which makes
VM_ForceSafepoint and VMThread accessible and doctoring the jni headers
to expose this extra API. Since the QUIT handling code forces a
safepoint synchronize, albeit at the extra cost of also doing a VM
thread dump, it did not seem worth the effort. If you disagree . . .

> Please, just lose all the #ifdef crap.  Either we believe this
> is a good patch and we should always use it or we should not
> commit this work.

Ok, will do.

>> +#ifdef USE_POLLING_PAGE_PROTECT
>> +// called from the SEGV handling code to see if a polling page read
>> +// is from a legitimate safepoint address
>> +int Thumb2_Check_Poll_Access(ucontext_t *uc, int magicByteOffset)
> 
> This is named wrongly; it doesn't just check, it adjusts the PC.
> 
> It's Thumb2_Poll_Maybe_Adjust_Return or somesuch.

Ok, I'll call it Thumb2_Install_Safepoint_PC()

> I'm not really convinced that all this magic word stuff is necessary.
> I guess if someone other than a safepoint check does fault we'll see
> it in the backtrace, so this is OK.  But is it really worth all the
> complexity?

If we remove this check and some bug happens to invalidly read or write
the safepoint page then we will blithely skip the PC forwards 6 or 8
bytes and return from the signal handler i.e. trying to continue
execution from wherever that adjustment takes us. We might be lucky and
see another SEGV or an  illegal instruction causing a fatal exit.
However, we might also be unlucky and continue Java execution with
corrupt Java or VM data. I would not want to debug the latter -- the
error might manifest many instructions later.

Of course, using a magic word is nto 100% bullet-proof. We might also
just happen to find the magic word when a rogue memory access hits the
polling page. However, this is extremely unlikely -- I chose 0xdead as
the value for THUMB2_POLLING_PAGE_MAGIC because it is an illegal thumb2
instruction.

>> +  // with a negative offset the generated code will look like
>> +  //    movw r_tmp, #polling_page
>> +  //    movt r_tmp, #polling_page
> 
> We do it twice?  What for?

Note this is the generated code. It requires two x 4 byte thumb2
instructions to load a 32-bit literal -- n.b. one of those is a movW and
one a movT.

>> +  // i.e. the generated coede includes the branch backwards twice
> 
> speling

Thank you.

> Otherwise OK.

Thank you for the feedback. I will correct it ASAP and repost.

regards,


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