Fix 15413: setRange for views with overlapping buffers.

runtime/lib/typed_data.dart

 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 // patch classes for Int8List ..... Float64List and ByteData implementations.
 
 patch class Int8List {
   /* patch */ factory Int8List(int length) {
     return new _Int8Array(length);
   }
@@ skipping 44 matching lines @@
       result[i] = elements[i];
     }
     return result;
   }
 
   /* patch */ factory Uint8ClampedList.view(ByteBuffer buffer,
                                             [int offsetInBytes = 0,
                                              int length]) {
     return new _Uint8ClampedArrayView(buffer, offsetInBytes, length);
   }
+
+  bool _isClamped() { return true; }
 }
 
 
 patch class Int16List {
   /* patch */ factory Int16List(int length) {
     return new _Int16Array(length);
   }
 
   /* patch */ factory Int16List.fromList(List<int> elements) {
     var result = new _Int16Array(elements.length);
@@ skipping 441 matching lines @@
     _rangeCheck(this.length, start, length);
     List result = _createList(length);
     result.setRange(0, length, this, start);
     return result;
   }
 
   Iterable getRange(int start, [int end]) {
     return IterableMixinWorkaround.getRangeList(this, start, end);
   }
 
-  void setRange(int start, int end, Iterable iterable, [int skipCount = 0]) {
-    if (!_setRange(start, end - start, iterable, skipCount)) {
-      IterableMixinWorkaround.setRangeList(this, start,
-                                           end, iterable, skipCount);
+  bool _isClamped() { return false; }
+
+  void setRange(int start, int end, Iterable from, [int skipCount = 0]) {
+    if (from is _TypedListBase){
+      final needsClamping =
+          this._isClamped() && (this._isClamped() != from._isClamped());
+      if (!needsClamping &&
+          (this.elementSizeInBytes == from.elementSizeInBytes)) {
+        if (this.buffer._setRange(

[sra1, 2014/01/07 18:55:48]

this.buffer is a ByteBuffer.
There is no guarantee that is is a typed list.

Even this.buffer it is a typed list, this could be a Int8List view of a
Float32List.

I have created a test for underlying-buffer-has-different-element-size-to-view,
which fails on the VM.

[srdjan, 2014/01/07 19:20:35]

OK.

On 2014/01/07 18:55:48, sra1 wrote:

+              start + (this.offsetInBytes ~/ this.elementSizeInBytes),
+              end - start,
+              from.buffer,
+              skipCount + (from.offsetInBytes  ~/ from.elementSizeInBytes))) {
+          return;
+        }
+      } else if (from.buffer == this.buffer) {
+        // Different element sizes, but same buffer means that we need
+        // an intermediate structure.

[sra1, 2014/01/07 18:55:48]

You never need an intermediate structure.
You might need two loops going in different directions.

In setRange_3_test, the first copy needs
(1) an up-loop copying a to aaaa, b to bbbb, c to cccc and d to dddd
(2) a down-loop copying h to hhhh, g to gggg, f to ffff and e to eeee

The point of switch-over is the element that is copied to the same address.
If there is no such address, then one loop is sufficient, but it might be the
up-loop or the down-loop.  This is just a more general case of choosing loop
direction in libc's memmove.

[srdjan, 2014/01/07 19:20:35]

Yes, next CL.

On 2014/01/07 18:55:48, sra1 wrote:

+        // TODO(srdjan): Optimize to skip copying if the range does not overlap.
+        final len = end - start;
+        final buffer = new List(len);
+        for (int i = 0; i < len; i++) {
+          buffer[i] = from[skipCount + i];
+        }
+        for (int i = start; i < end; i++) {
+          this[i] = buffer[i - start];
+        }
+        return;
+      }
     }
+    IterableMixinWorkaround.setRangeList(this, start,
+                                         end, from, skipCount);
   }
 
   void setAll(int index, Iterable iterable) {
     IterableMixinWorkaround.setAllList(this, index, iterable);
   }
 
   void fillRange(int start, int end, [fillValue]) {
     IterableMixinWorkaround.fillRangeList(this, start, end, fillValue);
   }
 
@@ skipping 19 matching lines @@
   }
 
   int get lengthInBytes {
     return length * elementSizeInBytes;
   }
 
   ByteBuffer get buffer {
     return this;
   }
 
-
   // Methods implementing the collection interface.
 
   int get length native "TypedData_length";
 
-
   // Internal utility methods.
 
   int _getInt8(int offsetInBytes) native "TypedData_GetInt8";
   void _setInt8(int offsetInBytes, int value) native "TypedData_SetInt8";
 
   int _getUint8(int offsetInBytes) native "TypedData_GetUint8";
   void _setUint8(int offsetInBytes, int value) native "TypedData_SetUint8";
 
   int _getInt16(int offsetInBytes) native "TypedData_GetInt16";
   void _setInt16(int offsetInBytes, int value) native "TypedData_SetInt16";
@@ skipping 145 matching lines @@
   }
 
   factory _Uint8ClampedArray.view(ByteBuffer buffer,
                                   [int offsetInBytes = 0, int length]) {
     if (length == null) {
       length = buffer.lengthInBytes - offsetInBytes;
     }
     return new _Uint8ClampedArrayView(buffer, offsetInBytes, length);
   }
 
+  bool _isClamped() { return true; }
 
   // Methods implementing List interface.
 
   int operator[](int index) {
     if (index < 0 || index >= length) {
       _throwRangeError(index, length);
     }
     return _getUint8(index);
   }
 
@@ skipping 745 matching lines @@
 }
 
 
 class _ExternalUint8ClampedArray extends _TypedList implements Uint8ClampedList {
   // Factory constructors.
 
   factory _ExternalUint8ClampedArray(int length) {
     return _new(length);
   }
 
+  bool _isClamped() { return true; }
 
   // Method(s) implementing the List interface.
 
   int operator[](int index) {
     if (index < 0 || index >= length) {
       _throwRangeError(index, length);
     }
     return _getUint8(index);
   }
 
@@ skipping 763 matching lines @@
 }
 
 
 class _TypedListView extends _TypedListBase implements TypedData {
   _TypedListView(ByteBuffer _buffer, int _offset, int _length)
     : _typedData = _buffer,  // This assignment is type safe.
       offsetInBytes = _offset,
       length = _length {
   }
 
-
   // Method(s) implementing the TypedData interface.
 
   int get lengthInBytes {
     return length * elementSizeInBytes;
   }
 
   ByteBuffer get buffer {
     return _typedData.buffer;
   }
 
@@ skipping 112 matching lines @@
     : super(buffer, _offsetInBytes,
             _defaultIfNull(_length,
                            ((buffer.lengthInBytes - _offsetInBytes) ~/
                             Uint8List.BYTES_PER_ELEMENT))) {
     _rangeCheck(buffer.lengthInBytes,
                 offsetInBytes,
                 length * Uint8List.BYTES_PER_ELEMENT);
   }
 
 
+  bool _isClamped() { return true; }
+
   // Method(s) implementing List interface.
 
   int operator[](int index) {
     if (index < 0 || index >= length) {
       _throwRangeError(index, length);
     }
     return _typedData._getUint8(offsetInBytes +
                                 (index * Uint8List.BYTES_PER_ELEMENT));
   }
 
@@ skipping 913 matching lines @@
     return value;
   }
   return object;
 }
 
 
 void _throwRangeError(int index, int length) {
   String message = "$index must be in the range [0..$length)";
   throw new RangeError(message);
 }