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: Failed null check at ClosureRepresentation.fieldIndexForSignature on a statically typed constructor tearoff expression #56372

Closed
plammens opened this issue Aug 4, 2024 · 15 comments
Labels
area-dart2wasm Issues for the dart2wasm compiler. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@plammens
Copy link

plammens commented Aug 4, 2024

I get a compilation error when compiling to wasm with flutter build web --wasm --release (that I don't get with other compilation targets.) (I use a flutter command but the error happens in the dart2wasm library, so I'm posting the issue here.)

The error is:

Target dart2wasm failed: ProcessException: Process exited abnormally with exit code 64:
*NOTE*: Compilation to WasmGC is experimental.
The support may change, or be removed, with no advance notice.

Exception in ConstantExpression at [...]:70:21

Unhandled exception:
Null check operator used on a null value

Stack trace:


#0      ClosureRepresentation.fieldIndexForSignature (package:dart2wasm/closures.dart:112)
#1      ConstantCreator.visitInstantiationConstant.<anonymous closure>.fillVtableEntry (package:dart2wasm/constants.dart:725)
#2      ConstantCreator.visitInstantiationConstant.<anonymous closure>.makeVtable (package:dart2wasm/constants.dart:749)
#3      ConstantCreator.visitInstantiationConstant.<anonymous closure> (package:dart2wasm/constants.dart:765)
#4      ConstantCreator.createConstant (package:dart2wasm/constants.dart:285)
#5      ConstantCreator.visitInstantiationConstant (package:dart2wasm/constants.dart:698)
#6      InstantiationConstant.accept (package:kernel/ast.dart:14209)
#7      ConstantCreator.ensureConstant (package:dart2wasm/constants.dart:252)
#8      ConstantInstantiator.defaultConstant (package:dart2wasm/constants.dart:137)
#9      ConstantVisitorDefaultMixin.visitStaticTearOffConstant (package:kernel/visitor.dart:944)
#10     InstantiationConstant.accept (package:kernel/ast.dart:14209)
#11     ConstantInstantiator.instantiate (package:dart2wasm/constants.dart:121)
#12     Constants.instantiateConstant (package:dart2wasm/constants.dart:103)
#13     CodeGenerator.visitConstantExpression (package:dart2wasm/code_generator.dart:2841)
#14     ConstantExpression.accept1 (package:kernel/ast.dart:8707)
#15     CodeGenerator.wrap (package:dart2wasm/code_generator.dart:852)
#16     CodeGenerator.visitRecordLiteral (package:dart2wasm/code_generator.dart:3086)
#17     RecordLiteral.accept1 (package:kernel/ast.dart:8490)
#18     CodeGenerator.wrap (package:dart2wasm/code_generator.dart:852)
#19     CodeGenerator.visitLet (package:dart2wasm/code_generator.dart:1669)
#20     Let.accept1 (package:kernel/ast.dart:8775)
#21     CodeGenerator.wrap (package:dart2wasm/code_generator.dart:852)
#22     CodeGenerator.visitLet (package:dart2wasm/code_generator.dart:1669)
#23     Let.accept1 (package:kernel/ast.dart:8775)
#24     CodeGenerator.wrap (package:dart2wasm/code_generator.dart:852)
#25     CodeGenerator.visitMapLiteral.<anonymous closure> (package:dart2wasm/code_generator.dart:2956)
#26     Translator.makeArray (package:dart2wasm/translator.dart:1025)
#27     CodeGenerator.makeArray (package:dart2wasm/code_generator.dart:2921)
#28     CodeGenerator.visitMapLiteral (package:dart2wasm/code_generator.dart:2949)
#29     MapLiteral.accept1 (package:kernel/ast.dart:8341)
#30     CodeGenerator.wrap (package:dart2wasm/code_generator.dart:852)
#31     CodeGenerator.visitVariableDeclaration (package:dart2wasm/code_generator.dart:1020)
#32     VariableDeclaration.accept (package:kernel/ast.dart:10791)
#33     CodeGenerator.visitStatement (package:dart2wasm/code_generator.dart:863)
#34     CodeGenerator.visitBlock (package:dart2wasm/code_generator.dart:983)
#35     Block.accept (package:kernel/ast.dart:9247)
#36     CodeGenerator.visitStatement (package:dart2wasm/code_generator.dart:863)
#37     CodeGenerator.generateBody (package:dart2wasm/code_generator.dart:557)
#38     CodeGenerator.generate (package:dart2wasm/code_generator.dart:236)
#39     Translator.translate (package:dart2wasm/translator.dart:374)
#40     compileToModule (package:dart2wasm/compile.dart:188)
<asynchronous suspension>
#41     generateWasm (package:dart2wasm/generate_wasm.dart:24)
<asynchronous suspension>
#42     main (file:///C:/b/s/w/ir/x/w/sdk/pkg/dart2wasm/bin/dart2wasm.dart:10)
<asynchronous suspension>

I've tried to write a minimal reproducible example from scratch, but I haven't succeeded, so I'll show my actual code.

The culprit of this exception, as pointed out by the error, seems to be this tearoff expression:

final Map<Type, _TypeConfig> typesConfig = {
      // [...]
      Habit: (
        enabled: isSubtype<Habit, C>(),
        typeInfo: CommitmentTypeInfo(
          name: "Habit",
          icon: Habit.icon_,
          orderKey: DiscreteHabit.commitmentTypeInfo.orderKey,
          fromMapFactory: (map) => throw UnimplementedError(),
        ),
        editScreen: EditHabit.blank   // ERROR: this is the constant expression that causes the error
      ),
     // [...]
    };

I have the following typedefs:

typedef _EditPageBuilder<C extends Commitment> = Widget Function({
  CompositeCommitment<C>? parentCommitment,
  Function(C)? onEdited,
});

typedef _TypeConfig<C extends Commitment> = ({
  bool enabled,
  CommitmentTypeInfo typeInfo,
  _EditPageBuilder<C> editScreen,
});

The problem disappears when I set:

typedef _EditPageBuilder<C extends Commitment> = dynamic;

Related code (definition of EditHabit.blank):

class EditHabit<H extends Habit> extends EditCommitment<H> {
  // [...]

  EditHabit.blank({
    super.key,
    super.parentCommitment,
    super.onEdited,
  }) : super.blank(
          commitmentTypeInfo: DiscreteHabit.commitmentTypeInfo,
          titleHintText: titleHint,
        );

  // [...]
}

abstract class EditCommitment<C extends Commitment> extends Edit<C> {
  // [...]

  /// For creating a new commitment.
  const EditCommitment.blank({
    super.key,
    required CommitmentTypeInfo commitmentTypeInfo,
    required super.titleHintText,
    this.parentCommitment,
    super.onEdited,
  })  : _commitmentTypeInfo = commitmentTypeInfo,
        super.blank();

  // Should be CompositeCommitment<C> but Dart forces the type parameter to be
  // covariant, which is not desirable here.
  final CompositeCommitment? parentCommitment;

  // [...]
}

abstract class Edit<O extends CommitmentStoreObjectSnapshot>
    extends ConsumerStatefulWidget {
  // [...]

  const Edit.blank({
    super.key,
    required this.titleHintText,
    this.onEdited,
  }) : object = null;

  /// Called after the edit/creation has been successfully executed.
  final Function(O)? onEdited;

  // [...]
}

Surprisingly, I have other elements in the map above for which this error does not occur, in particular for EditTask.blank:

class EditTask extends EditCommitment<Task> {
  // [...]

  const EditTask.blank({
    super.parentCommitment,
    super.key,
    super.onEdited,
  }) : super.blank(
          commitmentTypeInfo: Task.commitmentTypeInfo,
          titleHintText: titleHint,
        );

  // [...]
}

I thought the difference might be in the const, but I haven't managed to make a minimal reproducible example. Maybe someone experienced with dart2wasm will be able to do so.


  • Dart version and tooling diagnostic info (dart info)
#### General info

- Dart 3.4.4 (stable) (Wed Jun 12 15:54:31 2024 +0000) on "windows_x64"
- on windows / "Windows 10 Home" 10.0 (Build 22631)
- locale is en-GB

#### Project info

- sdk constraint: '^3.4.0'
- dependencies: built_collection, cloud_firestore, collection, cupertino_icons, easy_debounce, equatable, firebase_auth, firebase_core, firebase_ui_auth, firebase_ui_oauth_google, flut
ter, flutter_fancy_tree_view, flutter_local_notifications, flutter_localizations, flutter_riverpod, flutter_speed_dial, flutter_timezone, google_sign_in, intersperse, intl, meta, modal_bottom_sheet, package_info_plus, quiver, rrule_generator, skeletonizer, sliver_tools, stream_transform, timezone, url_launcher
- dev_dependencies: custom_lint, flutter_launcher_icons, flutter_lints, flutter_test, riverpod_lint, test
- elided dependencies: 2

#### Process info

| Memory | CPU | Elapsed time | Command line |
| -----: | --: | -----------: | ------------ |
| 274 MB |  -- |              | dart.exe     |
|  30 MB |  -- |              | dart.exe     |
|   3 MB |  -- |              | dart.exe     |
| 476 MB |  -- |              | dart.exe     |
| 485 MB |  -- |              | dart.exe     |
|  94 MB |  -- |              | dart.exe     |
| 100 MB |  -- |              | dart.exe     |

  • Whether you are using Windows, macOS, or Linux (if applicable)
    Windows

  • Whether you are using Chrome, Safari, Firefox, Edge (if applicable)
    Chrome

@plammens plammens changed the title dart2wasm: Failed null check at ClosureRepresentation.fieldIndexForSignature dart2wasm: Failed null check at ClosureRepresentation.fieldIndexForSignature on a statically typed constructor tearoff expression Aug 4, 2024
@dart-github-bot
Copy link
Collaborator

Summary: The dart2wasm compiler fails to compile a statically typed constructor tearoff expression used in a Map literal, resulting in a null check error during constant evaluation. The issue occurs when compiling to WasmGC and is not observed with other compilation targets.

@dart-github-bot dart-github-bot added area-dart2wasm Issues for the dart2wasm compiler. triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Aug 4, 2024
@lrhn lrhn removed the triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. label Aug 4, 2024
@mkustermann
Copy link
Member

/cc @osa1 Could you have a look?

@osa1 osa1 self-assigned this Aug 6, 2024
@osa1
Copy link
Member

osa1 commented Aug 6, 2024

I've tried to write a minimal reproducible example from scratch, but I haven't succeeded, so I'll show my actual code.

I'm unable to reproduce the issue from the snippets. Is it possible to share the whole project with commands to run to get the error?

@plammens
Copy link
Author

plammens commented Aug 7, 2024

I'm unable to reproduce the issue from the snippets. Is it possible to share the whole project with commands to run to get the error?

It's a private repo, maybe I can add you as a collaborator temporarily so you can access it?

@osa1
Copy link
Member

osa1 commented Aug 7, 2024

@plammens unfortunately we are only allowed to look at appropriately licensed open source code.

(Not sure which licenses are those exactly, but common ones like BSD, MIT etc. work)

For now I will try a few more things to try to reproduce this.

@mkustermann
Copy link
Member

@plammens Could you try to verify that this problem still happens when you're on flutter master channel?

The problem disappears when I set:

typedef _EditPageBuilder<C extends Commitment> = dynamic;

This is an indication that the issue is due to a partial instantiation of a tear-off constant, i.e. EditHabit is generic and will get its type argument filled in when EditHabit.blank is used in a context where one doesn't expect a generic function but one without type arguments, the language type inference.

I thought the difference might be in the const, but I haven't managed to make a minimal reproducible example. Maybe someone experienced with dart2wasm will be able to do so

The issue has to do with closure representations. Dart2wasm will try to make a good calling convention for closures. To do that it will look at a) all function definitions b) all function call sites. It then partitions them together into groups that have to be together (all possible definitions for a given callsite, all callsites for a definition) and based on those groups makes optimized vtables for fast closure calls.

If this issue still persists on flutter master channel, then we'll need a minimal reproduction we can look at (I've spent the entire morning looking through the relevant code and trying to construct cases that could trigger this, but failed to come up with an example where this would trigger).

What we need for a minimal reproduction is the code that creates those closures (class hierarchy, constructors and how the constructor tear-offs are used) and the code that invokes those closures. @plammens Could you try to make a minimal flutter app that reproduces this?

@plammens
Copy link
Author

plammens commented Aug 8, 2024

@plammens Could you try to verify that this problem still happens when you're on flutter master channel?

I can confirm that the issue is still there on master, flutter/flutter@e66ce15 .

```text Flutter 3.24.0-1.0.pre.517 • channel master • https://github.com/flutter/flutter.git Framework • revision e66ce1591f (16 minutes ago) • 2024-08-08 14:25:19 -0400 Engine • revision 387f6f3c5f Tools • Dart 3.6.0 (build 3.6.0-129.0.dev) • DevTools 2.38.0 ```

What we need for a minimal reproduction is the code that creates those closures (class hierarchy, constructors and how the constructor tear-offs are used) and the code that invokes those closures. @plammens Could you try to make a minimal flutter app that reproduces this?

I'll try again making a reproducible example, this time I'll try to work my way down from my actual code to a more minimal version instead of trying to build my way up.

@plammens
Copy link
Author

plammens commented Aug 9, 2024

I've used a script to put all my code in a single file to try to whittle it down to a minimal example, but it seems this is going to take forever because many times I delete some code that seems completely unrelated and the error goes away. Plus I have to wait 60 seconds between each iteration due to having to run the command. At least it does seem to be deterministic with respect to the code.

Currently gone from 10k lines of code to 7.5k. Is there any chance I can just send you the file privately and slap an open-source license on it? It's a personal project anyway.

@mkustermann
Copy link
Member

I've used a script to put all my code in a single file to try to whittle it down to a minimal example, but it seems this is going to take forever because many times I delete some code that seems completely unrelated and the error goes away. Plus I have to wait 60 seconds between each iteration due to having to run the command.

We really appreciate the effort, @plammens !

Currently gone from 10k lines of code to 7.5k. Is there any chance I can just send you the file privately and slap an open-source license on it? It's a personal project anyway.

If it's under an open source license we can take a look - irrespective of how it's shared. You're welcome to share via email.

@plammens
Copy link
Author

plammens commented Aug 9, 2024

If it's under an open source license we can take a look - irrespective of how it's shared. You're welcome to share via email.

Okay, I've added a GPL license to the file. What email address should I send it to? I'll also send the pubspec in case you need it.

By the way, don't try to make too much sense of the code as it is now in a heavily mutilated state :)

@mkustermann
Copy link
Member

Okay, I've added a GPL license to the file. What email address should I send it to? I'll also send the pubspec in case you need it.

You can send it to [email protected]

@plammens
Copy link
Author

You can send it to [email protected]

Done, thanks!

@mkustermann
Copy link
Member

mkustermann commented Aug 12, 2024

@plammens Thank you very much for the reproduction!

The following test case reproduces the issue

void main() {
  final funs = <String Function({Object? shared})>[
    foo,
    bar,
  ];

  final one = int.parse('1');
  final barS = funs[one] as String Function({Object? barSpecific});
  if (barS(barSpecific: 1) != 'bar(null, 1)') {
    throw 'failed: ${barS(barSpecific: 1)}';
  }
}

String foo<T>({Object? shared, Object? fooSpecific}) =>
    'foo<$T>($shared, $fooSpecific)';

String bar({Object? shared, Object? barSpecific}) =>
    'bar($shared, $barSpecific)';

=> Sent out cl/380100

copybara-service bot pushed a commit that referenced this issue Aug 12, 2024
Partial instantiation constants are closures. Those closures have
vtables with all entries needed for the closure representation
corresponding to the instantiated closure.

The entries of those vtables have to either call the corresponding
method of the generic closure, or are unreachable dummy entries.

Dummy entries can be required due to clustering callees/callers together
where a particular target doesn't support the name combination.

The case that was incorrect is if the particular name combination did
not get clustered with anything for the generic closure representation.

Issue #56372

TEST=web/wasm/regress_56372_test

Change-Id: Ifbf624e10dd1162f4d5660b43914e5b34ba82294
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/380100
Commit-Queue: Martin Kustermann <[email protected]>
Reviewed-by: Ömer Ağacan <[email protected]>
@osa1 osa1 removed their assignment Aug 13, 2024
@osa1
Copy link
Member

osa1 commented Aug 13, 2024

Fixed with f7fc4d0.

@osa1 osa1 closed this as completed Aug 13, 2024
@mkustermann
Copy link
Member

mkustermann commented Aug 13, 2024

@plammens The fix should have rolled up all the way to flutter/flutter now, so it's available on main/master channel

copybara-service bot pushed a commit that referenced this issue Aug 13, 2024
Partial instantiation constants are closures. Those closures have
vtables with all entries needed for the closure representation
corresponding to the instantiated closure.

The entries of those vtables have to either call the corresponding
method of the generic closure, or are unreachable dummy entries.

Dummy entries can be required due to clustering callees/callers together
where a particular target doesn't support the name combination.

The case that was incorrect is if the particular name combination did
not get clustered with anything for the generic closure representation.

Issue #56372

TEST=web/wasm/regress_56372_test

Bug: #56372
Change-Id: I7a219237519c39d982b89ce272f33fb4d90cd173
Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/380100
Cherry-pick-request: #56440
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/380120
Reviewed-by: Ömer Ağacan <[email protected]>
Commit-Queue: Martin Kustermann <[email protected]>
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. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

5 participants