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

VM Compiler crash on invalid Dart code (generic type) #34291

Closed
matanlurey opened this Issue Aug 28, 2018 · 6 comments

Comments

@matanlurey
Copy link
Contributor

matanlurey commented Aug 28, 2018

An internal user on Flutter hit the following compiler crash (partially obsfucated):

I was creating an image test file where I mistakenly included a generic type on a non-generic object. I had something like this:

  @override
  Widget build(BuildContext context) {
    return new Directionality(
        textDirection: textDirection,
        child: new charts.LineChart(
          seriesList,
          animate: false,
          // Configures four [ChartTitle] behaviors to render titles in each
          // chart margin.
          behaviors: [
            new charts.ChartTitle<num>('Top title text',
                subTitle: includeSubTitle ? 'Top sub-title text' : null,
                behaviorPosition: charts.BehaviorPosition.top,
                titleOutsideJustification: outsideJustification),
          ],
        ));
  }

charts.ChartTitle is not generic, so the mistaken copy-pasta <num> bit caused the following inscrutable error to occur. The stack trace doesn't contain any of my code, so it took a bit to work out exactly where the problem was. Now that I know the cause, I can infer that NamedTypeError is probably a reasonable type of error for the error. It would, however, be helpful to have a line number in my code to point the way.

The issue, here of course, is the compiler should never crash when given invalid Dart code.

Unhandled exception:
Crash when compiling package:flutter/material.dart,
at character offset null:
type 'QualifiedName' is not a subtype of type 'String'
#0      NamedTypeBuilder.check (package:front_end/src/fasta/builder/named_type_builder.dart:90:15)
#1      UnresolvedType.checkType (package:front_end/src/fasta/builder/unresolved_type.dart:21:31)
#2      SourceLibraryBuilder.resolveTypes (package:front_end/src/fasta/source/source_library_builder.dart:729:11)
#3      SourceLoader.resolveTypes.<anonymous closure> (package:front_end/src/fasta/source/source_loader.dart:377:36)
#4      __InternalLinkedHashMap&_HashVMBase&MapMixin&_LinkedHashMapMixin.forEach (dart:collection/runtime/libcompact_hash.dart:365:8)
#5      SourceLoader.resolveTypes (package:front_end/src/fasta/source/source_loader.dart:374:14)
#6      KernelTarget.buildOutlines.<anonymous closure> (package:front_end/src/fasta/kernel/kernel_target.dart:248:18)
...

Assigning to @kmillikin for area-front-end, and /cc @a-siva.

@matanlurey matanlurey changed the title VM/Flutter: Compiler crash on invalid Dart code (generic type) VM Compiler crash on invalid Dart code (generic type) Aug 28, 2018

@a-siva

This comment has been minimized.

Copy link
Contributor

a-siva commented Aug 29, 2018

/cc @aam

@dgrove dgrove added this to the Dart2.1 milestone Aug 29, 2018

@kmillikin

This comment has been minimized.

Copy link
Member

kmillikin commented Aug 30, 2018

I can't reproduce this with a simple attempt:

// in 34291.dart
import 'charts.dart' as charts;                                                                               
                                                                                                              
main() {                                                                                                      
  var c = new charts.ChartTitle<num>('text');                                                                 
} 
// in charts.dart
class ChartTitle {
  ChartTitle(String text);
}
$ pkg/front_end/tool/fasta compile --dump-ir --strong ~/programs/dart/34291.dart
library;                                                                                                      
import self as self;                                                                                          
import "./charts.dart" as cha;                                                                                
                                                                                                              
static method main() → dynamic {                                                                              
  cha::ChartTitle c = new cha::ChartTitle::•("text");                                                         
}                                                                                                             
library;                                                                                                      
import self as self;                                                                                          
import "dart:core" as core;                                                                                   
                                                                                                              
class ChartTitle extends core::Object {                                                                       
  constructor •(core::String text) → void                                                                     
    : super core::Object::•()                                                                                 
    ;                                                                                                         
}

@dart-lang dart-lang deleted a comment from matanlurey Aug 30, 2018

@aam

This comment has been minimized.

Copy link
Contributor

aam commented Aug 30, 2018

The issue seems to be due to QualifiedName passed as a String to templateTypeArgumentMismatch.withArguments, so the fix seems to be simple:

diff --git a/pkg/front_end/lib/src/fasta/builder/named_type_builder.dart b/pkg/front_end/lib/src/fasta/builder/named_type_builder.dart
index 08b77d8840..e4945fc551 100644
--- a/pkg/front_end/lib/src/fasta/builder/named_type_builder.dart
+++ b/pkg/front_end/lib/src/fasta/builder/named_type_builder.dart
@@ -87,7 +87,7 @@ abstract class NamedTypeBuilder<T extends TypeBuilder, R> extends TypeBuilder {
           charOffset,
           fileUri,
           templateTypeArgumentMismatch.withArguments(
-              name, declaration.typeVariablesCount));
+              name.toString(), declaration.typeVariablesCount));
     }
   }

Only thing that is remaining is to have a test that induces this error.

@peter-ahe-google

This comment has been minimized.

Copy link
Contributor

peter-ahe-google commented Aug 31, 2018

I'm currently reworking QualifiedName, and I'll be sure to fix this at the same time.

@peter-ahe-google

This comment has been minimized.

Copy link
Contributor

peter-ahe-google commented Sep 5, 2018

This was fixed in 015840d.

This is a standalone reproduction which I'll submit as a regression test:

==> lib.dart <==
class A {}

==> t.dart <==
import "lib.dart" as lib;

class B {}

lib.A<B> foo() {}

main() {}
@peter-ahe-google

This comment has been minimized.

Copy link
Contributor

peter-ahe-google commented Sep 5, 2018

dart-bot pushed a commit that referenced this issue Sep 5, 2018

Regression test for issue 34291
See #34291

Change-Id: I3a3d621efac939a8005707283c1d7105c7d4d8b4
Reviewed-on: https://dart-review.googlesource.com/73140
Reviewed-by: Jens Johansen <jensj@google.com>
Commit-Queue: Peter von der Ahé <ahe@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.