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

[Christian Thalinger]

Looks good.

On Oct 25, 2013, at 9:46 AM, Mikael Gerdin <mikael.gerdin at oracle.com> wrote:

> 
> 
> 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
>> 
> 
>