Review/Comment request (S) 8027252: Crash in interpreter because get_unsigned_2_byte_index_at_bcp reads 4 bytes

[Mikael Gerdin]

On 25 October 2013 18:25:47 Christian Thalinger 
<christian.thalinger at oracle.com> wrote:
> Good catch.  Is there a similar issue on SPARC?

I looked through the other platforms, they all seem to do single-byte loads 
for the indices.
On SPARC I suppose it's because it can't do unaligned multibyte loads.
/Mikael

>
> On Oct 25, 2013, at 8:55 AM, Mikael Gerdin <mikael.gerdin at oracle.com> wrote:
>
> > Hi all,
> > Initially I'd like to hear people's opinions on this issue. It appears to 
> have surfaced after we've reduced the unnecessary alignment "cushions" for 
> metaspace together with the fact that we are now always able to use the 
> very last bit of a VirtualSpace (in Metaspace).
> > The issue is extremely difficult to reproduce, because it relies on the 
> exact order and size of all metaspace allocations for a run to trigger this 
> problem.
> > Description of the problem:
> > We recently had some mysterious crashes in the GC testing, and I narrowed 
> them down to the following scenario:
> > Imagine that we have a VirtualSpace (for Metaspace)
> > [                     |        bb]
> >                       ConstMethod
> > And that the very last object allocated in such a space is a ConstMethod, 
> and that it has its embedded bytecodes filling up to the very end.
> > Consider the case when the second last bytecode (the one before 
> "Xreturn") is one with an embedded 2-byte index (a constant pool or local 
> variable index) for example:
> > - (bcp) checkcast #6 0xc0 0x00 0x06 - (bcp+3) areturn 0xb0 - (bcp+4) 
> UNMAPPED MEMORY
> > The template interpreter here uses a 4-byte load on intel to read the 
> index embedded in the checkcast:
> > mov    0x1(%esi),%ebx
> > this will read the bytes [bcp+1,bcp+4] and cause a SEGV.
> > I looked through the code for places where we do similar loads of 2-byte 
> indexes and found the same problem for iload_w:
> >   0xe73f7fbb:  mov    0x2(%esi),%ebx
> >   0xe73f7fbe:  bswap  %ebx
> >   0xe73f7fc0:  shr    $0x10,%ebx
> > I looked through most places where we emit
> > __ movl
> > __ bswap
> > and changed the places I could find which were affected.
> > Suggested fix:
> > Replace movl with load_[un]signed_short in the places where we load 
> 2-byte values from the byte code stream.
> > I've verified that load_unsigned_short (which boils down to movzwl, 
> opcodes 0x0f 0xb7 only reads 2 bytes).
> > Buglink: https://bugs.openjdk.java.net/browse/JDK-8027252
> > Webrev: http://cr.openjdk.java.net/~mgerdin/8027252/webrev.0/
> > Testing:
> > vm.quick.testlist -Xint on linux-i586 and x86_64(running)
> > Thanks
> > /Mikael
>