RFR(s): 8076185: Provide SafeFetchX implementation for zero

[Severin Gehwolf]

Hi Thomas,

I think it should be fine to keep the jump buffer on the stack. Thanks
for the numbers!

Cheers,
Severin

On Mon, 2015-03-30 at 14:32 +0200, Thomas Stüfe wrote:
> Hi Severin,
> 
> 
> thanks for testing! I am pretty sure the coding is POSIX compliant and
> therefore should work fine on all POSIX platforms.
> 
> 
> If stack space is a problem, jump buffer could be moved to C-Heap.
> However, this might not be necessary. I checked some sizes on some
> platforms:
> 
> 
> x64 linux
> ucontext_t 936
> sigjmp_buf 200
> 
> 
> ia64 linux
> ucontext_t 2656
> sigjmp_buf 704
> 
> 
> ia64 hpux
> ucontext_t 48640 
> sigjmp_buf 1280  
> 
> 
> power AIX
> ucontext_t 688
> sigjmp_buf 680
> 
> 
> largest sigjmp_buf I saw was HP-UX with 1.2K. I think we routinely do
> worse than that e.g. during error reporting.
> 
> 
> (Note, in comparison, that ucontext_t can be monstrous :-) - but then,
> it wants to do much more than unwinding the stack in a single thread).
> 
> 
> So, advantage of less stack is offset by performance (malloc) and
> potentially errors (if C heap is corrupted we might still want to do
> SafeFetch()). I don't know. I think leaving that thing on the stack is
> fine unless you want very small stacks.
> 
> 
> Kind Regards, Thomas
> 
> 
> 
> 
> 
> 
>  
> 
> 
> 
> 
> 
> 
> 
> On Mon, Mar 30, 2015 at 1:07 PM, Severin Gehwolf <sgehwolf at redhat.com>
> wrote:
>         Hi Thomas,
>         
>         On Fri, 2015-03-27 at 19:00 +0100, Thomas Stüfe wrote:
>         > Hi all,
>         >
>         >
>         > Please review this change which provides a real SafeFetch
>         > implementation
>         > on zero.
>         >
>         >
>         > webrev:
>         >
>         http://cr.openjdk.java.net/~stuefe/webrevs/8076185/webrev.01/webrev/
>         > bug:     https://bugs.openjdk.java.net/browse/JDK-8076185
>         >
>         >
>         > It works like this:
>         >
>         >
>         > Before a load is attempted from a potentially unsafe
>         address, we set
>         > up a
>         > jump buffer with sigsetjmp(). In the signal handler, for
>         SIGSEGV and
>         > SIGBUS,
>         > we test whether there is a jump buffer set and if yes, take
>         this as an
>         > indication
>         > that the crash was an attempted SafeFetch. In this case we
>         jump back
>         > via longjmp().
>         >
>         >
>         > Coding is a bit more difficult because we need to be
>         threadsafe. We
>         > keep the jump
>         > buffer on the thread stack - this is ok, because that stack
>         never gets
>         > unwinded
>         >  - either we crash, in which case signal handler stack
>         frames are
>         > built up below
>         > us, or we don't crash, in which case we never leave the
>         SafeFetch
>         > function. In
>         > both cases, we never loose the jump buffer.
>         >
>         >
>         > To communicate the jump buffer location to the signal
>         handler, TLS is
>         > used. I
>         > use POSIX tls, because that always works and is not
>         dependend on VM
>         > infrastructure.
>         >
>         >
>         > ---
>         >
>         >
>         > I built and tested this on Linux x64 zero (Ubuntu 14.4). It
>         works and
>         > the
>         > initialization tests for SafeFetch in the stub routine
>         initialization
>         > now work for
>         > zero too.
>         >
>         >
>         > Note that I do not have a BSD system right now, so I cannot
>         check
>         > whether this
>         > change builds and works for BSD. But the change only
>         requires POSIX
>         > Apis, so
>         > BSD should probably be ok.
>         >
>         >
>         > Someone from the zero team should definitly check and test
>         this for
>         > other zero
>         > platforms, but the chances are good that this just works.
>         
>         
>         Thomas, thanks very much for this patch! It works fine for me
>         on x86_64
>         F21, but I'd also like to test on non-x86. Zero is notoriously
>         stack
>         hungry and having the jump buffer on the stack too is a bit of
>         a concern
>         (but probably fine). Testing that too, though, will take a
>         while, since
>         I don't have such hardware readily available.
>         
>         As for the patch (I'm not a reviewer): It looks fine to me.
>         It's
>         certainly a lot better than this unsafe SafeFetch that we
>         currently use
>         for Zero.
>         
>         Thanks,
>         Severin
>         
>         > Thanks and Kind Regards,
>         >
>         >
>         > Thomas Stuefe
>         >
>         >
>         
>         
>         
>         
> 
>