Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[dart2wasm] compiler crashes on an inequality operator #53070

Closed
yjbanov opened this issue Jul 28, 2023 · 4 comments
Closed

[dart2wasm] compiler crashes on an inequality operator #53070

yjbanov opened this issue Jul 28, 2023 · 4 comments
Labels
area-dart2wasm Issues for the dart2wasm compiler.

Comments

@yjbanov
Copy link

yjbanov commented Jul 28, 2023

Repro

  • Clone this commit: yjbanov/engine@3ca695f
  • cd $FLUTTER_ENGINE_SRC/flutter/lib/web_ui
  • dev/felt test --verbose --browser=chrome --compiler=dart2wasm --renderer=html test/engine/pointer_binding_test.dart

Expected

The program should compile and the test should pass.

Actual

dart2wasm crashes with the following:

Exception in EqualsCall at file:///Users/yjbanov/code/flutter/engine/src/flutter/lib/web_ui/lib/src/engine/pointer_binding.dart:352:47
Unhandled exception:
Null check operator used on a null value
#0      Translator.translateStorageType (package:dart2wasm/translator.dart:545)
#1      Translator.translateType (package:dart2wasm/translator.dart:462)
#2      Intrinsifier.typeOfExp (package:dart2wasm/intrinsics.dart:141)
#3      Intrinsifier.generateEqualsIntrinsic (package:dart2wasm/intrinsics.dart:508)
#4      CodeGenerator.visitEqualsCall (package:dart2wasm/code_generator.dart:1678)
#5      EqualsCall.accept1 (package:kernel/ast.dart:6300)
#6      CodeGenerator.wrap (package:dart2wasm/code_generator.dart:633)
#7      CodeGenerator.visitNot (package:dart2wasm/code_generator.dart:2480)
#8      Not.accept1 (package:kernel/ast.dart:6806)
#9      CodeGenerator.wrap (package:dart2wasm/code_generator.dart:633)
#10     CodeGenerator.visitVariableDeclaration (package:dart2wasm/code_generator.dart:797)
#11     VariableDeclaration.accept (package:kernel/ast.dart:10709)
#12     CodeGenerator.visitStatement (package:dart2wasm/code_generator.dart:644)
#13     CodeGenerator.visitBlock (package:dart2wasm/code_generator.dart:748)
#14     Block.accept (package:kernel/ast.dart:9173)
#15     CodeGenerator.visitStatement (package:dart2wasm/code_generator.dart:644)
#16     CodeGenerator.visitIfStatement.<anonymous closure> (package:dart2wasm/code_generator.dart:1168)
#17     CodeGenerator._conditional (package:dart2wasm/code_generator.dart:1140)
#18     CodeGenerator.visitIfStatement (package:dart2wasm/code_generator.dart:1166)
#19     IfStatement.accept (package:kernel/ast.dart:10064)
#20     CodeGenerator.visitStatement (package:dart2wasm/code_generator.dart:644)
#21     CodeGenerator.visitBlock (package:dart2wasm/code_generator.dart:748)
#22     Block.accept (package:kernel/ast.dart:9173)
#23     CodeGenerator.visitStatement (package:dart2wasm/code_generator.dart:644)
#24     CodeGenerator.generateBody (package:dart2wasm/code_generator.dart:452)
#25     CodeGenerator.generate (package:dart2wasm/code_generator.dart:224)
#26     Translator.translate (package:dart2wasm/translator.dart:346)
#27     compileToModule (package:dart2wasm/compile.dart:129)
<asynchronous suspension>
#28     main (package:dart2wasm/dart2wasm.dart:134)
<asynchronous suspension>
#29     main (file:///opt/s/w/ir/x/w/sdk/pkg/dart2wasm/bin/dart2wasm.dart:7)
<asynchronous suspension>
ERROR: Failed to compile test /Users/yjbanov/code/flutter/engine/src/flutter/lib/web_ui/test/engine/pointer_binding_test.dart. dart2wasm exited with exit code 255

The line of code in question is https://github.com/yjbanov/engine/blob/3ca695f5a1c861b6e1f62a6afddc45cfef2c60ae/lib/web_ui/lib/src/engine/pointer_binding.dart#L352

Workaround

Rewrite this line:

final bool targetChanged = event.target != state.target;

To this (i.e. extract the inner expressions into variables):

final DomEventTarget? eventTarget = event.target;
final DomElement stateTarget = state.target;
final bool targetChanged = eventTarget != stateTarget;

This makes the compiler happy and it outputs seemingly correct wasm (the test passes). However, the rewrite shouldn't be necessary.

@osa1 osa1 added the area-dart2wasm Issues for the dart2wasm compiler. label Jul 28, 2023
@osa1
Copy link
Member

osa1 commented Jul 29, 2023

Smaller repro:

import 'dart:js_interop';

@JS()
@staticInterop
class DomEventTarget {}

typedef Testing = ({
  DomEventTarget a,
  int b,
});

Testing f() {
  throw '';
}

void main() {
  var x = f();
  var y = f();
  print(x.a == y.a);
}

@osa1
Copy link
Member

osa1 commented Jul 29, 2023

I think this is a TFA bug. The class DomEventTarget above is not added to the Library so dart2wasm doesn't create a ClassInfo for it. If I add @pragma('wasm:entry-point') to DomEventTarget then it's added to the Library and the code compiles fine. @alexmarkov could you take a look?

@lrhn
Copy link
Member

lrhn commented Jul 29, 2023

I guess analysis is smart enough to see that no instance of DomEvenTarget is ever created, so the class can be tree shaken.

Someone then needs to also recognize that code which appears to have an instance of that type, must be unreachable. And not try to create code calling an instance method of the type.

@alexmarkov
Copy link
Contributor

On a small example, kernel before TFA:

  import "dart:js_interop";

  typedef Testing = ({required a: _js::JSValue, required b: core::int});
  @#C2
  @#C3
  class DomEventTarget extends core::Object {
    synthetic constructor •() → foo::DomEventTarget
      : super core::Object::•()
      ;
    static method _#new#tearOff() → _js::JSValue
      return new foo::DomEventTarget::•() as{ForLegacy} _js::JSValue;
  }
  static method f() → ({required a: _js::JSValue, required b: core::int}) {
    throw "";
  }
  static method main() → void {
    ({required a: _js::JSValue, required b: core::int}) x = foo::f();
    ({required a: _js::JSValue, required b: core::int}) y = foo::f();
    core::print(x.a{foo::DomEventTarget} =={core::Object::==}{(core::Object) → core::bool} y.a{foo::DomEventTarget});
  }

Kernel after TFA:

  import "dart:js_interop";

  static method f() → ({required a: _js::JSValue, required b: core::int}) {
    throw "";
  }
  static method main() → void {
    ({required a: _js::JSValue, required b: core::int}) x = [@vm.inferred-type.metadata=!] foo::f();
    ({required a: _js::JSValue, required b: core::int}) y = [@vm.inferred-type.metadata=!] foo::f();
    block {
      [@vm.inferred-type.metadata=! (skip check) (receiver not int)] x.a{foo::DomEventTarget} =={core::Object::==}{(core::Object) → core::bool} y.a{foo::DomEventTarget};
    } =>throw "Attempt to execute code removed by Dart AOT compiler (TFA)";
  }

Indeed, TFA inferred that DomEventTarget is never allocated. Furthermore, dart2wasm replaced DomEventTarget with JSValue in certain types before TFA, so TFA inferred that DomEventTarget is not used in types and removed it entirely.

It looks like kernel before TFA is incorrectly typed as DomEventTarget is replaced with JSValue in variable type but not static receiver type of RecordNameGet nodes. This happens because RecordNameGet.receiverType is not visited in the AST visitors. Missing visitor is also the reason why TFA overlooks that DomEventTarget is used in that particular type.

https://dart-review.googlesource.com/c/sdk/+/317142 should fix this bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-dart2wasm Issues for the dart2wasm compiler.
Projects
None yet
Development

No branches or pull requests

4 participants