Skip to content

VM: report error if input .dill file is missing the sdk and --kernel-binaries was not provided #30151

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

Closed
peter-ahe-google opened this issue Jul 13, 2017 · 6 comments
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends.

Comments

@peter-ahe-google
Copy link
Contributor

$ ./pkg/front_end/tool/fasta compile pkg/front_end/testcases/hello.dart
$ ./xcodebuild/ReleaseX64/dart pkg/front_end/testcases/hello.dart.dill 
$

No output, and no error. It should have printed "Hello, World!"

If I add --compile-sdk it works fine:

$ ./pkg/front_end/tool/fasta compile --compile-sdk=xcodebuild/ReleaseX64/patched_sdk/ pkg/front_end/testcases/hello.dart
$ ./xcodebuild/ReleaseX64/dart pkg/front_end/testcases/hello.dart.dill 
Hello, World!
$ 
@peter-ahe-google peter-ahe-google added the legacy-area-front-end Legacy: Use area-dart-model instead. label Jul 13, 2017
@sigmundch
Copy link
Member

Note that another solution is to give the VM the kernel platform file as follows:

./xcodebuild/ReleaseX64/dart --kernel-binaries=xcodebuild/ReleaseX64/patched_sdk/ pkg/front_end/testcases/hello.dart.dill 

I agree that we need better error messages here. In this case I believe the error should be reported by the VM.

When the VM sees that it only has "external" library references for the core libraries, it needs to issue an error suggesting that the user provide the kernel-binaries folder.

@sigmundch sigmundch removed their assignment Jul 13, 2017
@sigmundch sigmundch added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. and removed legacy-area-front-end Legacy: Use area-dart-model instead. labels Jul 13, 2017
@sigmundch sigmundch changed the title pkg/front_end/tool/fasta compile doesn't generate runnable .dill file anymore VM: report error if input .dill file is missing the sdk and --kernel-binaries was not provided Jul 17, 2017
@sivachandra
Copy link
Contributor

@sigmundch @peter-ahe-google

There is probably something I am missing, but I am unable to fasta compile a dart file with a command like this:

out/ReleaseX64/dart-sdk/lib/front_end/tool/fasta compile pkg/front_end/testcases/hello.dart

I see this error:

FileSystemException: Cannot open file, path = '/usr/local/google/home/sivachandra/dart-sdk/sdk/platform.dill' (OS Error: No such file or directory, errno = 2)
#0      _File.throwIfError (dart:io/file_impl.dart:599)
#1      _File.openSync (dart:io/file_impl.dart:454)
#2      _File.readAsBytesSync (dart:io/file_impl.dart:514)
#3      _appendDillForUri (package:front_end/src/fasta/fasta.dart:233:37)
#4      CompileTask.buildOutline (package:front_end/src/fasta/fasta.dart:139:7)
<asynchronous suspension>
#5      CompileTask.compile (package:front_end/src/fasta/fasta.dart:162:39)
<asynchronous suspension>
#6      compile.<anonymous closure> (package:front_end/src/fasta/fasta.dart:100:25)
<asynchronous suspension>
#7      CompilerContext.withGlobalOptions.<anonymous closure> (package:front_end/src/fasta/compiler_context.dart:76:33)
#8      _rootRun (dart:async/zone.dart:1120)
#9      _CustomZone.run (dart:async/zone.dart:1001)
#10     runZoned (dart:async/zone.dart:1467)
#11     CompilerContext.withGlobalOptions (package:front_end/src/fasta/compiler_context.dart:76:12)
#12     CompilerCommandLine.withGlobalOptions (package:front_end/src/fasta/compiler_command_line.dart:160:28)
#13     compile (package:front_end/src/fasta/fasta.dart:93:38)
<asynchronous suspension>
#14     compileEntryPoint (package:front_end/src/fasta/fasta.dart:50:11)
<asynchronous suspension>
#15     main (file:///usr/local/google/home/sivachandra/dart-sdk/sdk/pkg/front_end/tool/_fasta/compile.dart:7:33)
#16     _startIsolate.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:263)
#17     _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:151)

My usual command works:

./out/ReleaseX64/dart-sdk/lib/front_end/tool/fasta compile --platform=out/ReleaseX64/patched_sdk/platform.dill pkg/front_end/testcases/hello.dart

@sivachandra
Copy link
Contributor

BTW, I am not blocked as I can reproduce Peter's original problem by linking to outline.dill.

I have a question though: Should we be supporting such a use case at all? That is, should one be able to execute a .dill file directly? If yes, then at the least .dill files should be versioned IMO.

@sigmundch
Copy link
Member

Sorry for the confusion on the fasta compile script. It internally infers a location for platform.dill if one wasn't provided using some heuristics. The heuristics depend on what Dart binary you use to run the script, so the small differences of how you and Peter invoked fasta might explain why you get that error.

Good question about versioning, I agree - it would be nice to report a friendly error message if your .dill file version doesn't match the VM's. Am I right that you already do this for snapshots? I filed #30188 to track this.

In terms of the use cases we should support, I'd like to hear @kmillikin thinking on this too.

My take is that we do want to support 3 ways to run kernel in the VM:

  1. loading a .dill file containing the script and the full platform code
  2. loading a .dill file containing the script and an outline of platform code marked as external
  3. send binary file (not .dill) containing the script and dangling references to platform code

I expect (2) will be a supported way of providing kernel to the VM by users, but (3) is only going to be supported for our internal tools (kernel-service and hot-reload use this format).

So I think for this specific bug, we mainly need to address (2).

As I write this here I realized now that the repro above is not quite accurate: fasta compile is generating the script with the outline, but the outline is not marked external, so in a way it makes sense that hello world doesn't print: the print message is an empty function.

We'll fix fasta-compile to produce the right output, but meanwhile, if you need to have that external marker, then you might need to add one line of code to fasta (like this)

@sivachandra
Copy link
Contributor

@sigmundch: For the question about snapshot versioning, yes we do that already. And, thanks a lot for the detailed response.

@a-siva
Copy link
Contributor

a-siva commented Feb 7, 2018

We don't have a --kernel-binaries option anymore and so this bug is obsolete, platform dill is part of the VM and will always be found.

@a-siva a-siva closed this as completed Feb 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends.
Projects
None yet
Development

No branches or pull requests

4 participants