RFR 8162458 Buffer view implementations use incorrect offset for Unsafe access

[Paul Sandoz]

Hooking in nio dev.

I think this issue is important to review/push soon (i.e. this week) so as it does not leak out beyond the hs repo.

—

Incidentally this patch also fixes what might have been a long standing bug in the compact method of buffer views.

Here is ByteBufferAsIntBufferB’s compact method:

  public IntBuffer compact() {

      int pos = position();
      int lim = limit();
      assert (pos <= lim);
      int rem = (pos <= lim ? lim - pos : 0);

      ByteBuffer db = bb.duplicate();
      db.limit(ix(lim));
      db.position(ix(0));
      ByteBuffer sb = db.slice();
      sb.position(pos << 2);
      sb.compact();
      position(rem);
      limit(capacity());
      discardMark();
      return this;
  }


For view heap buffers the ix method used to return an offset relative to the underlying byte array, not relative to the buffer. this this can result values that are beyond the capacity of the buffer.

The following code:

  ByteBuffer bb0 = ByteBuffer.allocate(16);
  ByteBuffer bb4 = bb0.position(4).slice();
  ByteBuffer bb8 = bb4.position(4).slice();
  IntBuffer ib = bb8.asIntBuffer().compact();

results in:

  Exception in thread "main" java.lang.IllegalArgumentException: newLimit > capacity: (16 > 8)

Paul.


> On 27 Jul 2016, at 13:47, Paul Sandoz <Paul.Sandoz at oracle.com> wrote:
> 
> Hi,
> 
> I made an embarrassing mistake in the fix for
> 
>  https://bugs.openjdk.java.net/browse/JDK-8151163
>  All Buffer implementations should leverage Unsafe unaligned accessors
> 
> The offset calculation for Unsafe access was incorrect, it’s easy to get confused because for heap buffers the offset is relative to the array, and for direct buffers the address (which can update for slices/duplicates). Disturbingly all existing tests were passing both for core and hotspot when i pushed to hs.
> 
> As a penance i wrote a combinatorial test for buffer views to navigate the twisty maze of heap/direct, aligned/unaligned, little/big endian for accessing binary data and views from the source buffer.
> 
> Please review:
> 
>  http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8162458-byte-buffer-view-offset-access-incorrect/webrev/
> 
> (This may be a duplicate of [1]).
> 
> Test has been verified to fail with the existing code. Focused JPRT runs pass, but i will kick off core/hotspot runs later on today.
> 
> I will push to hs since that is where JDK-8151163 and it has not been integrated into jdk9/dev.
> 
> Paul.
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8159257
> unsafe.cpp: assert(byte_offset < p_size) failed: Unsafe access: offset 32767 > object's size 16
> 
> For the test runtime/Unsafe/RangeCheck.java I can reproduce a crash in jdk9/dev which does not have JDK-8151163, and i can reproduce on jdk9/hs with this fix for JDK-8162458.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 841 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://mail.openjdk.java.net/pipermail/nio-dev/attachments/20160728/cc5f3a09/signature.asc>