Flutter VM bug

[jslavitz]

I'm having problems with a really odd bug where, in hooks.dart, in the function _unpackPointerDataPacket, if there are more than 13 64bit objects being unpacked, the last object will, after roughly 400 calls, return 0 (or some other number that's incorrect) instead of the value passed into the function. I don't have a compact test case, but if you check out these two commits below, you can see the issue. Feel free to dm me on chat for more information. Thanks!

engine: flutter/engine@d1617d2
framework: flutter/flutter@4507aaf

Flutter doctor:
[✓] Flutter (Channel unknown, v0.5.8-pre.286, on Mac OS X 10.13.6 17G65, locale en-US) [✓] Android toolchain - develop for Android devices (Android SDK 28.0.1) [✓] iOS toolchain - develop for iOS devices (Xcode 9.4) [✓] Android Studio (version 3.1) [✓] IntelliJ IDEA Community Edition (version 2018.1) [!] VS Code (version 1.27.1) [✓] Connected devices (1 available)

To see the bug, you will need to run the flutter gallery and scroll with a mouse scroll wheel.

[jonahwilliams]

For reference, the code itself is doing some fairly trivial reads. In this case, the last value read from the buffer will eventually become an arbitrary small double value. We've definitely spent some time looking over to code and neither Jeremy nor I have been able to identify any obvious errors here.

PointerDataPacket _unpackPointerDataPacket(ByteData packet) {
  const int kStride = Int64List.bytesPerElement;
  const int kBytesPerPointerData = _kPointerDataFieldCount * kStride;
  final int length = packet.lengthInBytes ~/ kBytesPerPointerData;
  assert(length * kBytesPerPointerData == packet.lengthInBytes);
  final List<PointerData> data = new List<PointerData>(length);
  for (int i = 0; i < length; ++i) {
    int offset = i * _kPointerDataFieldCount;
    data[i] = new PointerData(
      timeStamp: new Duration(microseconds: packet.getInt64(kStride * offset++, _kFakeHostEndian)),
      change: PointerChange.values[packet.getInt64(kStride * offset++, _kFakeHostEndian)],
      kind: PointerDeviceKind.values[packet.getInt64(kStride * offset++, _kFakeHostEndian)],
      device: packet.getInt64(kStride * offset++, _kFakeHostEndian),
      physicalX: packet.getFloat64(kStride * offset++, _kFakeHostEndian),
      physicalY: packet.getFloat64(kStride * offset++, _kFakeHostEndian),
      buttons: packet.getInt64(kStride * offset++, _kFakeHostEndian),
      obscured: packet.getInt64(kStride * offset++, _kFakeHostEndian) != 0,
      pressure: packet.getFloat64(kStride * offset++, _kFakeHostEndian),
      pressureMin: packet.getFloat64(kStride * offset++, _kFakeHostEndian),
      pressureMax: packet.getFloat64(kStride * offset++, _kFakeHostEndian),
      distance: packet.getFloat64(kStride * offset++, _kFakeHostEndian),
      distanceMax: packet.getFloat64(kStride * offset++, _kFakeHostEndian),
      radiusMajor: packet.getFloat64(kStride * offset++, _kFakeHostEndian),
      radiusMinor: packet.getFloat64(kStride * offset++, _kFakeHostEndian),
      radiusMin: packet.getFloat64(kStride * offset++, _kFakeHostEndian),
      radiusMax: packet.getFloat64(kStride * offset++, _kFakeHostEndian),
      orientation: packet.getFloat64(kStride * offset++, _kFakeHostEndian),
      tilt: packet.getFloat64(kStride * offset++, _kFakeHostEndian)
    );
    assert(offset == (i + 1) * _kPointerDataFieldCount);
  }
  return new PointerDataPacket(data: data);
}

This behavior is only observed in the DartVM in debug builds and not on release builds.

[Hixie]

Is there a reduced test case?

[jslavitz]

No, is there a good way to run a code snippet on the vm?

[dnfield]

As long as it happens on the host engine, you could write a unit test in the engine for it.

[mraleph]

This sounds like a JIT compiler issue to me. I will take a look at this.

[mraleph]

The issue reproduces just by extracting the code from Flutter and running it on ARM:

import 'dart:typed_data';

/// How the pointer has changed since the last report.
enum PointerChange {
  /// The input from the pointer is no longer directed towards this receiver.
  cancel,

  /// The device has started tracking the pointer.
  ///
  /// For example, the pointer might be hovering above the device, having not yet
  /// made contact with the surface of the device.
  add,

  /// The device is no longer tracking the pointer.
  ///
  /// For example, the pointer might have drifted out of the device's hover
  /// detection range or might have been disconnected from the system entirely.
  remove,

  /// The pointer has moved with respect to the device while not in contact with
  /// the device.
  hover,

  /// The pointer has made contact with the device.
  down,

  /// The pointer has moved with respect to the device while in contact with the
  /// device.
  move,

  /// The pointer has stopped making contact with the device.
  up,
}

/// The kind of pointer device.
enum PointerDeviceKind {
  /// A touch-based pointer device.
  touch,

  /// A mouse-based pointer device.
  mouse,

  /// A pointer device with a stylus.
  stylus,

  /// A pointer device with a stylus that has been inverted.
  invertedStylus,

  /// An unknown pointer device.
  unknown
}

/// Information about the state of a pointer.
class PointerData {
  /// Creates an object that represents the state of a pointer.
  const PointerData({
    this.timeStamp: Duration.zero,
    this.change: PointerChange.cancel,
    this.kind: PointerDeviceKind.touch,
    this.device: 0,
    this.physicalX: 0.0,
    this.physicalY: 0.0,
    this.buttons: 0,
    this.obscured: false,
    this.pressure: 0.0,
    this.pressureMin: 0.0,
    this.pressureMax: 0.0,
    this.distance: 0.0,
    this.distanceMax: 0.0,
    this.radiusMajor: 0.0,
    this.radiusMinor: 0.0,
    this.radiusMin: 0.0,
    this.radiusMax: 0.0,
    this.orientation: 0.0,
    this.tilt: 0.0
  });

  /// Time of event dispatch, relative to an arbitrary timeline.
  final Duration timeStamp;

  /// How the pointer has changed since the last report.
  final PointerChange change;

  /// The kind of input device for which the event was generated.
  final PointerDeviceKind kind;

  /// Unique identifier for the pointing device, reused across interactions.
  final int device;

  /// X coordinate of the position of the pointer, in physical pixels in the
  /// global coordinate space.
  final double physicalX;

  /// Y coordinate of the position of the pointer, in physical pixels in the
  /// global coordinate space.
  final double physicalY;

  /// Bit field using the *Button constants (primaryMouseButton,
  /// secondaryStylusButton, etc). For example, if this has the value 6 and the
  /// [kind] is [PointerDeviceKind.invertedStylus], then this indicates an
  /// upside-down stylus with both its primary and secondary buttons pressed.
  final int buttons;

  /// Set if an application from a different security domain is in any way
  /// obscuring this application's window. (Aspirational; not currently
  /// implemented.)
  final bool obscured;

  /// The pressure of the touch as a number ranging from 0.0, indicating a touch
  /// with no discernible pressure, to 1.0, indicating a touch with "normal"
  /// pressure, and possibly beyond, indicating a stronger touch. For devices
  /// that do not detect pressure (e.g. mice), returns 1.0.
  final double pressure;

  /// The minimum value that [pressure] can return for this pointer. For devices
  /// that do not detect pressure (e.g. mice), returns 1.0. This will always be
  /// a number less than or equal to 1.0.
  final double pressureMin;

  /// The maximum value that [pressure] can return for this pointer. For devices
  /// that do not detect pressure (e.g. mice), returns 1.0. This will always be
  /// a greater than or equal to 1.0.
  final double pressureMax;

  /// The distance of the detected object from the input surface (e.g. the
  /// distance of a stylus or finger from a touch screen), in arbitrary units on
  /// an arbitrary (not necessarily linear) scale. If the pointer is down, this
  /// is 0.0 by definition.
  final double distance;

  /// The maximum value that a distance can return for this pointer. If this
  /// input device cannot detect "hover touch" input events, then this will be
  /// 0.0.
  final double distanceMax;

  /// The radius of the contact ellipse along the major axis, in logical pixels.
  final double radiusMajor;

  /// The radius of the contact ellipse along the minor axis, in logical pixels.
  final double radiusMinor;

  /// The minimum value that could be reported for radiusMajor and radiusMinor
  /// for this pointer, in logical pixels.
  final double radiusMin;

  /// The minimum value that could be reported for radiusMajor and radiusMinor
  /// for this pointer, in logical pixels.
  final double radiusMax;

  /// For PointerDeviceKind.touch events:
  ///
  /// The angle of the contact ellipse, in radius in the range:
  ///
  ///    -pi/2 < orientation <= pi/2
  ///
  /// ...giving the angle of the major axis of the ellipse with the y-axis
  /// (negative angles indicating an orientation along the top-left /
  /// bottom-right diagonal, positive angles indicating an orientation along the
  /// top-right / bottom-left diagonal, and zero indicating an orientation
  /// parallel with the y-axis).
  ///
  /// For PointerDeviceKind.stylus and PointerDeviceKind.invertedStylus events:
  ///
  /// The angle of the stylus, in radians in the range:
  ///
  ///    -pi < orientation <= pi
  ///
  /// ...giving the angle of the axis of the stylus projected onto the input
  /// surface, relative to the positive y-axis of that surface (thus 0.0
  /// indicates the stylus, if projected onto that surface, would go from the
  /// contact point vertically up in the positive y-axis direction, pi would
  /// indicate that the stylus would go down in the negative y-axis direction;
  /// pi/4 would indicate that the stylus goes up and to the right, -pi/2 would
  /// indicate that the stylus goes to the left, etc).
  final double orientation;

  /// For PointerDeviceKind.stylus and PointerDeviceKind.invertedStylus events:
  ///
  /// The angle of the stylus, in radians in the range:
  ///
  ///    0 <= tilt <= pi/2
  ///
  /// ...giving the angle of the axis of the stylus, relative to the axis
  /// perpendicular to the input surface (thus 0.0 indicates the stylus is
  /// orthogonal to the plane of the input surface, while pi/2 indicates that
  /// the stylus is flat on that surface).
  final double tilt;

  @override
  String toString() => '$runtimeType(x: $physicalX, y: $physicalY)';

  /// Returns a complete textual description of the information in this object.
  String toStringFull() {
    return '$runtimeType('
             'timeStamp: $timeStamp, '
             'change: $change, '
             'kind: $kind, '
             'device: $device, '
             'physicalX: $physicalX, '
             'physicalY: $physicalY, '
             'buttons: $buttons, '
             'pressure: $pressure, '
             'pressureMin: $pressureMin, '
             'pressureMax: $pressureMax, '
             'distance: $distance, '
             'distanceMax: $distanceMax, '
             'radiusMajor: $radiusMajor, '
             'radiusMinor: $radiusMinor, '
             'radiusMin: $radiusMin, '
             'radiusMax: $radiusMax, '
             'orientation: $orientation, '
             'tilt: $tilt'
           ')';
  }
}

/// A sequence of reports about the state of pointers.
class PointerDataPacket {
  /// Creates a packet of pointer data reports.
  const PointerDataPacket({ this.data: const <PointerData>[] });

  /// Data about the individual pointers in this packet.
  ///
  /// This list might contain multiple pieces of data about the same pointer.
  final List<PointerData> data;
}

const int _kPointerDataFieldCount = 19;
const Endian _kFakeHostEndian = Endian.little;

PointerDataPacket _unpackPointerDataPacket(ByteData packet) {
  const int kStride = Int64List.bytesPerElement;
  const int kBytesPerPointerData = _kPointerDataFieldCount * kStride;
  final int length = packet.lengthInBytes ~/ kBytesPerPointerData;
  assert(length * kBytesPerPointerData == packet.lengthInBytes);
  final List<PointerData> data = new List<PointerData>(length);
  for (int i = 0; i < length; ++i) {
    int offset = i * _kPointerDataFieldCount;
    data[i] = new PointerData(
      timeStamp: new Duration(microseconds: packet.getInt64(kStride * offset++, _kFakeHostEndian)),
      change: PointerChange.values[packet.getInt64(kStride * offset++, _kFakeHostEndian)],
      kind: PointerDeviceKind.values[packet.getInt64(kStride * offset++, _kFakeHostEndian)],
      device: packet.getInt64(kStride * offset++, _kFakeHostEndian),
      physicalX: packet.getFloat64(kStride * offset++, _kFakeHostEndian),
      physicalY: packet.getFloat64(kStride * offset++, _kFakeHostEndian),
      buttons: packet.getInt64(kStride * offset++, _kFakeHostEndian),
      obscured: packet.getInt64(kStride * offset++, _kFakeHostEndian) != 0,
      pressure: packet.getFloat64(kStride * offset++, _kFakeHostEndian),
      pressureMin: packet.getFloat64(kStride * offset++, _kFakeHostEndian),
      pressureMax: packet.getFloat64(kStride * offset++, _kFakeHostEndian),
      distance: packet.getFloat64(kStride * offset++, _kFakeHostEndian),
      distanceMax: packet.getFloat64(kStride * offset++, _kFakeHostEndian),
      radiusMajor: packet.getFloat64(kStride * offset++, _kFakeHostEndian),
      radiusMinor: packet.getFloat64(kStride * offset++, _kFakeHostEndian),
      radiusMin: packet.getFloat64(kStride * offset++, _kFakeHostEndian),
      radiusMax: packet.getFloat64(kStride * offset++, _kFakeHostEndian),
      orientation: packet.getFloat64(kStride * offset++, _kFakeHostEndian),
      tilt: packet.getFloat64(kStride * offset++, _kFakeHostEndian)
    );
    assert(offset == (i + 1) * _kPointerDataFieldCount);
  }
  return new PointerDataPacket(data: data);
}

Uint8List packPointerDataPacket(PointerDataPacket packet) {
  const int kStride = Int64List.bytesPerElement;
  const int kBytesPerPointerData = _kPointerDataFieldCount * kStride;
  final result = new Uint8List(packet.data.length * kBytesPerPointerData);
  final buf = new ByteData.view(result.buffer);
  int i = 0;
  for (var data in packet.data) {
    int offset = i++ * _kPointerDataFieldCount;
    buf.setInt64(kStride * offset++, data.timeStamp.inMicroseconds, _kFakeHostEndian);
    buf.setInt64(kStride * offset++, data.change.index, _kFakeHostEndian);
    buf.setInt64(kStride * offset++, data.kind.index, _kFakeHostEndian);
    buf.setInt64(kStride * offset++, data.device, _kFakeHostEndian);
    buf.setFloat64(kStride * offset++, data.physicalX, _kFakeHostEndian);
    buf.setFloat64(kStride * offset++, data.physicalY, _kFakeHostEndian);
    buf.setInt64(kStride * offset++, data.buttons, _kFakeHostEndian);
    buf.setInt64(kStride * offset++, data.obscured ? 1 : 0, _kFakeHostEndian);
    buf.setFloat64(kStride * offset++, data.pressure, _kFakeHostEndian);
    buf.setFloat64(kStride * offset++, data.pressureMin, _kFakeHostEndian);
    buf.setFloat64(kStride * offset++, data.pressureMax, _kFakeHostEndian);
    buf.setFloat64(kStride * offset++, data.distance, _kFakeHostEndian);
    buf.setFloat64(kStride * offset++, data.distanceMax, _kFakeHostEndian);
    buf.setFloat64(kStride * offset++, data.radiusMajor, _kFakeHostEndian);
    buf.setFloat64(kStride * offset++, data.radiusMinor, _kFakeHostEndian);
    buf.setFloat64(kStride * offset++, data.radiusMin, _kFakeHostEndian);
    buf.setFloat64(kStride * offset++, data.radiusMax, _kFakeHostEndian);
    buf.setFloat64(kStride * offset++, data.orientation, _kFakeHostEndian);
    buf.setFloat64(kStride * offset++, data.tilt, _kFakeHostEndian);
  }
  return result;
}

void main() {
  final data = new List<PointerData>.generate(10, (i) => new PointerData(tilt: i.toDouble()));
  final packet = packPointerDataPacket(new PointerDataPacket(data: data));
  for (var i = 0; i < 100000; i++) {
    final unpacked = _unpackPointerDataPacket(new ByteData.view(packet.buffer)).data;
    for (var j = 0; j < unpacked.length; j++) {
      if (data[j].tilt != unpacked[j].tilt) {
        throw 'Failed on iteration #${i}: ${data[j].tilt} got ${unpacked[j].tilt} at index ${j}';
      }
    }
  }
}

╭─vegorov@volhv ~/src/dart/sdk ‹fix-build-crash›
╰─$ out/ReleaseX64/dart --enable_asserts /tmp/repro.dart
╭─vegorov@volhv ~/src/dart/sdk ‹fix-build-crash›
╰─$ out/ReleaseSIMARM/dart --enable_asserts /tmp/repro.dart
Unhandled exception:
Failed on iteration #44: 0.0 got 8974840.0 at index 0
#0      main (file:///tmp/repro.dart:303:9)
#1      _startIsolate.<anonymous closure> (dart:isolate/runtime/libisolate_patch.dart:289:19)
#2      _RawReceivePortImpl._handleMessage (dart:isolate/runtime/libisolate_patch.dart:171:12)

[mraleph]

From what I see we assemble the last load incorrectly (maybe some register indices overflowed?)

        ;; v1517 <- LoadIndexed(v224 T{_Uint8List}, v1493) T{_Double}
         ;; Inlined [_ByteDataView@6027147.getFloat64]
 0xf414a004    e08460c9               add r6, r4, r9, asr #1
 0xf414a008    e2866007               add r6, r6, #7
 0xf414a00c    e5d61000               ldrb r1, [r6, #+0]
 0xf414a010    e5d6c001               ldrb ip, [r6, #+1]
 0xf414a014    e181140c               orr r1, r1, ip, lsl #8
 0xf414a018    e5d6c002               ldrb ip, [r6, #+2]
 0xf414a01c    e181180c               orr r1, r1, ip, lsl #16
 0xf414a020    e5d6c003               ldrb ip, [r6, #+3]
 0xf414a024    e1811c0c               orr r1, r1, ip, lsl #24
 0xf414a028    ee1a1a10               vmovr r1, s20
 0xf414a02c    e2866004               add r6, r6, #4
 0xf414a030    e5d61000               ldrb r1, [r6, #+0]
 0xf414a034    e5d6c001               ldrb ip, [r6, #+1]
 0xf414a038    e181140c               orr r1, r1, ip, lsl #8
 0xf414a03c    e5d6c002               ldrb ip, [r6, #+2]
 0xf414a040    e181180c               orr r1, r1, ip, lsl #16
 0xf414a044    e5d6c003               ldrb ip, [r6, #+3]
 0xf414a048    e1811c0c               orr r1, r1, ip, lsl #24
 0xf414a04c    ee1a1a90               vmovr r1, s21

Will dig more into this next week.

Also notice that the code is pretty bad because we don't know if the source is aligned :-/ Might be good for code size and performance of this function to fix this up.

[mraleph]

I uploaded the fix: https://dart-review.googlesource.com/c/sdk/+/75021

However this made me review some of the code and I realized that our ARM backend has an issue with how it works with S FPU registers. I filed #34472 to track that.