Skip to content

Commit 7b50b7a

Browse files
alexmarkovcommit-bot@chromium.org
authored andcommitted
[vm/bytecode] Bootstrapping VM from bytecode
Previously, core snapshot was generated from AST (because --enable-interpreter/--use-bytecode-compiler was not specified when building core snapshot). As the result, CL https://dart.googlesource.com/sdk/+/da8cb470cc94830a98d49532e8d5d1a5b3d80f8b which declared libraries in bytecode also removed bytecode entirely from core snapshot in Dart SDK. This CL enables bytecode by default if --bytecode argument is specified for gn.py. This enables JIT compiler from bytecode (interpreter is still disabled by default but can be enabled using --enable-interpreter). Core snapshot and other snapshots now have bytecode. This change revealed a bunch of bugs which are fixed in this CL: * _Closure fields were treated as unboxing candidates which triggered assertion in LoadFieldTOS in interpreter. * Several places should load class declarations if they are not loaded yet. * Canonicalization of TypeRef objects which are not fully initialized may cause duplicate entries in the hash table of canonical TypeArguments. This triggers assertions when hash table is rehashed. The solution is to avoid canonicalization of non-root recursive types and recursive type arguments. Also, TypeRef::Canonicalize and TypeRef::Hash are reverted to assert and work only if type was set. * Native wrapper classes are eagerly stamped as type-finalized which caused assertion failures when reading their class declarations from bytecode. * When building flow graph for FFI trampolines kernel offset of library (which is now declared in bytecode) was queried. Added special case to Function::KernelDataProgramOffset(). * In interpreter-only mode with simulator (e.g. SIMARM64) if simulator is not called before code is interrupted with stack overflow check, simulator returns get_sp() = 0, which was treated as stack overflow. * test standalone_2/io/platform_resolved_executable_test.dart spawns sub-process but it didn't pass VM options. Change-Id: I81bc4f1a4c6725cfa246a435ebe5d8abe43abc67 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/107199 Commit-Queue: Alexander Markov <[email protected]> Reviewed-by: Ryan Macnak <[email protected]> Reviewed-by: Régis Crelier <[email protected]>
1 parent c1c24c2 commit 7b50b7a

11 files changed

Lines changed: 161 additions & 59 deletions

File tree

runtime/BUILD.gn

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,10 @@ config("dart_config") {
146146
include_dirs += [ "../third_party/tcmalloc/gperftools/src" ]
147147
}
148148

149+
if (dart_platform_bytecode) {
150+
defines += [ "DART_USE_BYTECODE" ]
151+
}
152+
149153
if (is_fuchsia) {
150154
if (using_fuchsia_sdk) {
151155
# TODO(chinmaygarde): Currenty these targets need to be build in the

runtime/vm/bootstrap.cc

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
#include "vm/bootstrap.h"
66

7+
#include <memory>
8+
79
#include "include/dart_api.h"
810

911
#include "vm/class_finalizer.h"
@@ -50,11 +52,18 @@ static void Finish(Thread* thread) {
5052
Class& cls = Class::Handle(zone, object_store->closure_class());
5153
ClassFinalizer::LoadClassMembers(cls);
5254

55+
// Make sure _Closure fields are not marked as unboxing candidates
56+
// as they are accessed with plain loads.
57+
const Array& fields = Array::Handle(zone, cls.fields());
58+
Field& field = Field::Handle(zone);
59+
for (intptr_t i = 0; i < fields.Length(); ++i) {
60+
field ^= fields.At(i);
61+
field.set_is_unboxing_candidate(false);
62+
}
63+
5364
#if defined(DEBUG)
5465
// Verify that closure field offsets are identical in Dart and C++.
55-
const Array& fields = Array::Handle(zone, cls.fields());
5666
ASSERT(fields.Length() == 6);
57-
Field& field = Field::Handle(zone);
5867
field ^= fields.At(0);
5968
ASSERT(field.Offset() == Closure::instantiator_type_arguments_offset());
6069
field ^= fields.At(1);

runtime/vm/class_finalizer.cc

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -195,14 +195,19 @@ bool ClassFinalizer::ProcessPendingClasses() {
195195
#if defined(DEBUG)
196196
for (intptr_t i = 0; i < class_array.Length(); i++) {
197197
cls ^= class_array.At(i);
198-
ASSERT(cls.is_declaration_loaded());
198+
ASSERT(cls.is_declared_in_bytecode() || cls.is_declaration_loaded());
199199
}
200200
#endif
201201

202202
// Finalize types in all classes.
203203
for (intptr_t i = 0; i < class_array.Length(); i++) {
204204
cls ^= class_array.At(i);
205-
FinalizeTypesInClass(cls);
205+
if (cls.is_declared_in_bytecode()) {
206+
cls.EnsureDeclarationLoaded();
207+
ASSERT(cls.is_type_finalized());
208+
} else {
209+
FinalizeTypesInClass(cls);
210+
}
206211
}
207212

208213
if (FLAG_print_classes) {
@@ -407,6 +412,7 @@ intptr_t ClassFinalizer::ExpandAndFinalizeTypeArguments(
407412
// The type class does not need to be finalized in order to finalize the type.
408413
// Also, the type parameters of the type class must be finalized.
409414
Class& type_class = Class::Handle(zone, type.type_class());
415+
type_class.EnsureDeclarationLoaded();
410416
if (!type_class.is_type_finalized()) {
411417
FinalizeTypeParameters(type_class, pending_types);
412418
}
@@ -1003,7 +1009,7 @@ static void MarkImplemented(Zone* zone, const Class& iface) {
10031009
void ClassFinalizer::FinalizeTypesInClass(const Class& cls) {
10041010
Thread* thread = Thread::Current();
10051011
HANDLESCOPE(thread);
1006-
ASSERT(cls.is_declaration_loaded());
1012+
cls.EnsureDeclarationLoaded();
10071013
if (cls.is_type_finalized()) {
10081014
return;
10091015
}
@@ -1190,12 +1196,7 @@ RawError* ClassFinalizer::LoadClassMembers(const Class& cls) {
11901196
LongJumpScope jump;
11911197
if (setjmp(*jump.Set()) == 0) {
11921198
#if !defined(DART_PRECOMPILED_RUNTIME)
1193-
if (!cls.is_declaration_loaded()) {
1194-
// Loading of class declaration can be postponed until needed
1195-
// if class comes from bytecode.
1196-
ASSERT(cls.is_declared_in_bytecode());
1197-
kernel::BytecodeReader::LoadClassDeclaration(cls);
1198-
}
1199+
cls.EnsureDeclarationLoaded();
11991200
#endif
12001201
ASSERT(cls.is_type_finalized());
12011202
ClassFinalizer::FinalizeClass(cls);

runtime/vm/compiler/frontend/bytecode_reader.cc

Lines changed: 80 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ void BytecodeMetadataHelper::ReadLibrary(const Library& library) {
137137
&Array::Handle(helper_->zone_, GetBytecodeComponent()));
138138
BytecodeReaderHelper bytecode_reader(&H, active_class_, &bytecode_component);
139139
AlternativeReadingScope alt(&bytecode_reader.reader(),
140-
library.bytecode_offset());
140+
bytecode_component.GetLibraryIndexOffset());
141141
bytecode_reader.FindAndReadSpecificLibrary(
142142
library, bytecode_component.GetNumLibraries());
143143
}
@@ -527,6 +527,35 @@ void BytecodeReaderHelper::ReadClosureDeclaration(const Function& function,
527527
closure.SetSignatureType(signature_type);
528528
}
529529

530+
static bool IsNonCanonical(const AbstractType& type) {
531+
return type.IsTypeRef() || (type.IsType() && !type.IsCanonical());
532+
}
533+
534+
static bool HasNonCanonicalTypes(Zone* zone, const Function& func) {
535+
auto& type = AbstractType::Handle(zone);
536+
for (intptr_t i = 0; i < func.NumParameters(); ++i) {
537+
type = func.ParameterTypeAt(i);
538+
if (IsNonCanonical(type)) {
539+
return true;
540+
}
541+
}
542+
type = func.result_type();
543+
if (IsNonCanonical(type)) {
544+
return true;
545+
}
546+
const auto& type_params = TypeArguments::Handle(zone, func.type_parameters());
547+
if (!type_params.IsNull()) {
548+
for (intptr_t i = 0; i < type_params.Length(); ++i) {
549+
type = type_params.TypeAt(i);
550+
type = TypeParameter::Cast(type).bound();
551+
if (IsNonCanonical(type)) {
552+
return true;
553+
}
554+
}
555+
}
556+
return false;
557+
}
558+
530559
RawType* BytecodeReaderHelper::ReadFunctionSignature(
531560
const Function& func,
532561
bool has_optional_positional_params,
@@ -581,7 +610,14 @@ RawType* BytecodeReaderHelper::ReadFunctionSignature(
581610

582611
// Finalize function type.
583612
type = func.SignatureType();
584-
type = ClassFinalizer::FinalizeType(*(active_class_->klass), type);
613+
ClassFinalizer::FinalizationKind finalization = ClassFinalizer::kCanonicalize;
614+
if (pending_recursive_types_ != nullptr && HasNonCanonicalTypes(Z, func)) {
615+
// This function type is a part of recursive type. Avoid canonicalization
616+
// as not all TypeRef objects are filled up at this point.
617+
finalization = ClassFinalizer::kFinalize;
618+
}
619+
type =
620+
ClassFinalizer::FinalizeType(*(active_class_->klass), type, finalization);
585621
return Type::Cast(type).raw();
586622
}
587623

@@ -1244,7 +1280,7 @@ RawObject* BytecodeReaderHelper::ReadObjectContents(uint32_t header) {
12441280
}
12451281
return cls;
12461282
}
1247-
RawClass* cls = library.LookupClassAllowPrivate(class_name);
1283+
RawClass* cls = library.LookupLocalClass(class_name);
12481284
NoSafepointScope no_safepoint_scope(thread_);
12491285
if (cls == Class::null()) {
12501286
FATAL2("Unable to find class %s in %s", class_name.ToCString(),
@@ -1581,10 +1617,7 @@ RawObject* BytecodeReaderHelper::ReadType(intptr_t tag) {
15811617
return AbstractType::void_type().raw();
15821618
case kSimpleType: {
15831619
const Class& cls = Class::CheckedHandle(Z, ReadObject());
1584-
if (!cls.is_declaration_loaded()) {
1585-
ASSERT(cls.is_declared_in_bytecode());
1586-
BytecodeReader::LoadClassDeclaration(cls);
1587-
}
1620+
cls.EnsureDeclarationLoaded();
15881621
return cls.DeclarationType();
15891622
}
15901623
case kTypeParameter: {
@@ -1615,10 +1648,7 @@ RawObject* BytecodeReaderHelper::ReadType(intptr_t tag) {
16151648
}
16161649
case kGenericType: {
16171650
const Class& cls = Class::CheckedHandle(Z, ReadObject());
1618-
if (!cls.is_declaration_loaded()) {
1619-
ASSERT(cls.is_declared_in_bytecode());
1620-
BytecodeReader::LoadClassDeclaration(cls);
1621-
}
1651+
cls.EnsureDeclarationLoaded();
16221652
const TypeArguments& type_arguments =
16231653
TypeArguments::CheckedHandle(Z, ReadObject());
16241654
const Type& type = Type::Handle(
@@ -1629,10 +1659,7 @@ RawObject* BytecodeReaderHelper::ReadType(intptr_t tag) {
16291659
case kRecursiveGenericType: {
16301660
const intptr_t id = reader_.ReadUInt();
16311661
const Class& cls = Class::CheckedHandle(Z, ReadObject());
1632-
if (!cls.is_declaration_loaded()) {
1633-
ASSERT(cls.is_declared_in_bytecode());
1634-
BytecodeReader::LoadClassDeclaration(cls);
1635-
}
1662+
cls.EnsureDeclarationLoaded();
16361663
const auto saved_pending_recursive_types = pending_recursive_types_;
16371664
if (id == 0) {
16381665
pending_recursive_types_ = &GrowableObjectArray::Handle(
@@ -1643,8 +1670,10 @@ RawObject* BytecodeReaderHelper::ReadType(intptr_t tag) {
16431670
TypeRef::Handle(Z, TypeRef::New(AbstractType::null_abstract_type()));
16441671
pending_recursive_types_->Add(type_ref);
16451672

1673+
reading_type_arguments_of_recursive_type_ = true;
16461674
const TypeArguments& type_arguments =
16471675
TypeArguments::CheckedHandle(Z, ReadObject());
1676+
reading_type_arguments_of_recursive_type_ = false;
16481677

16491678
ASSERT(id == pending_recursive_types_->Length() - 1);
16501679
ASSERT(pending_recursive_types_->At(id) == type_ref.raw());
@@ -1655,6 +1684,11 @@ RawObject* BytecodeReaderHelper::ReadType(intptr_t tag) {
16551684
Z, Type::New(cls, type_arguments, TokenPosition::kNoSource));
16561685
type_ref.set_type(type);
16571686
type.SetIsFinalized();
1687+
if (id != 0) {
1688+
// Do not canonicalize non-root recursive types
1689+
// as not all TypeRef objects are filled up at this point.
1690+
return type.raw();
1691+
}
16581692
return type.Canonicalize();
16591693
}
16601694
case kRecursiveTypeRef: {
@@ -1770,6 +1804,8 @@ RawScript* BytecodeReaderHelper::ReadSourceFile(const String& uri,
17701804
}
17711805

17721806
RawTypeArguments* BytecodeReaderHelper::ReadTypeArguments() {
1807+
const bool is_recursive = reading_type_arguments_of_recursive_type_;
1808+
reading_type_arguments_of_recursive_type_ = false;
17731809
const intptr_t length = reader_.ReadUInt();
17741810
TypeArguments& type_arguments =
17751811
TypeArguments::ZoneHandle(Z, TypeArguments::New(length));
@@ -1778,6 +1814,14 @@ RawTypeArguments* BytecodeReaderHelper::ReadTypeArguments() {
17781814
type ^= ReadObject();
17791815
type_arguments.SetTypeAt(i, type);
17801816
}
1817+
if (is_recursive) {
1818+
// Avoid canonicalization of type arguments of recursive type
1819+
// as not all TypeRef objects are filled up at this point.
1820+
// Type arguments will be canoncialized when the root recursive
1821+
// type is canonicalized.
1822+
ASSERT(pending_recursive_types_ != nullptr);
1823+
return type_arguments.raw();
1824+
}
17811825
return type_arguments.Canonicalize();
17821826
}
17831827

@@ -2221,13 +2265,17 @@ void BytecodeReaderHelper::ReadClassDeclaration(const Class& cls) {
22212265
// Its cid is set in Class::New / Isolate::RegisterClass /
22222266
// ClassTable::Register, unless it was loaded for expression evaluation.
22232267
ASSERT(cls.is_declared_in_bytecode());
2224-
ASSERT(!cls.is_declaration_loaded());
2268+
ASSERT(!cls.is_declaration_loaded() || loading_native_wrappers_library_);
22252269

22262270
const intptr_t flags = reader_.ReadUInt();
22272271
const bool has_pragma = (flags & kHasPragmaFlag) != 0;
22282272

22292273
// Set early to enable access to type_parameters().
2230-
cls.set_is_declaration_loaded();
2274+
// TODO(alexmarkov): revise early stamping of native wrapper classes
2275+
// as loaded.
2276+
if (!cls.is_declaration_loaded()) {
2277+
cls.set_is_declaration_loaded();
2278+
}
22312279

22322280
const auto& script = Script::CheckedHandle(Z, ReadObject());
22332281
cls.set_script(script);
@@ -2294,13 +2342,6 @@ void BytecodeReaderHelper::ReadClassDeclaration(const Class& cls) {
22942342

22952343
library.AddClassMetadata(cls, top_level_class, TokenPosition::kNoSource,
22962344
0, annotations_offset);
2297-
if (has_pragma) {
2298-
// TODO(alexmarkov): read annotations right away using
2299-
// annotations_offset.
2300-
NoOOBMessageScope no_msg_scope(thread_);
2301-
NoReloadScope no_reload_scope(thread_->isolate(), thread_);
2302-
library.GetMetadata(cls);
2303-
}
23042345
}
23052346
}
23062347
}
@@ -2310,7 +2351,11 @@ void BytecodeReaderHelper::ReadClassDeclaration(const Class& cls) {
23102351
bytecode_component_->GetMembersOffset());
23112352

23122353
// All types are finalized if loading from bytecode.
2313-
cls.set_is_type_finalized();
2354+
// TODO(alexmarkov): revise early stamping of native wrapper classes
2355+
// as type-finalized.
2356+
if (!cls.is_type_finalized()) {
2357+
cls.set_is_type_finalized();
2358+
}
23142359

23152360
// TODO(alexmarkov): move this to class finalization.
23162361
ClassFinalizer::RegisterClassInHierarchy(Z, cls);
@@ -2379,10 +2424,11 @@ void BytecodeReaderHelper::ReadLibraryDeclaration(const Library& library,
23792424
}
23802425
} else {
23812426
if (lookup_classes) {
2382-
cls = library.LookupClassAllowPrivate(name);
2427+
cls = library.LookupLocalClass(name);
23832428
}
23842429
if (lookup_classes && !cls.IsNull()) {
2385-
ASSERT(!cls.is_declaration_loaded());
2430+
ASSERT(!cls.is_declaration_loaded() ||
2431+
loading_native_wrappers_library_);
23862432
cls.set_script(script);
23872433
} else {
23882434
cls = Class::New(library, name, script, TokenPosition::kNoSource,
@@ -2395,6 +2441,13 @@ void BytecodeReaderHelper::ReadLibraryDeclaration(const Library& library,
23952441

23962442
cls.set_is_declared_in_bytecode(true);
23972443
cls.set_bytecode_offset(class_offset);
2444+
2445+
if (loading_native_wrappers_library_ || !register_class) {
2446+
AlternativeReadingScope alt(&reader_, class_offset);
2447+
ReadClassDeclaration(cls);
2448+
AlternativeReadingScope alt2(&reader_, cls.bytecode_offset());
2449+
ReadMembers(cls, /* discard_fields = */ false);
2450+
}
23982451
}
23992452

24002453
ASSERT(!library.Loaded());

runtime/vm/compiler/frontend/bytecode_reader.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,7 @@ class BytecodeReaderHelper : public ValueObject {
225225
Class& scoped_function_class_;
226226
Library* expression_evaluation_library_ = nullptr;
227227
bool loading_native_wrappers_library_ = false;
228+
bool reading_type_arguments_of_recursive_type_ = false;
228229

229230
DISALLOW_COPY_AND_ASSIGN(BytecodeReaderHelper);
230231
};

runtime/vm/dart_api_impl.cc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
#include "include/dart_api.h"
66
#include "include/dart_native_api.h"
77

8+
#include <memory>
9+
810
#include "lib/stacktrace.h"
911
#include "platform/assert.h"
1012
#include "vm/class_finalizer.h"
@@ -5133,6 +5135,7 @@ DART_EXPORT Dart_Handle Dart_GetType(Dart_Handle library,
51335135
return Api::NewError("Type '%s' not found in library '%s'.",
51345136
name_str.ToCString(), lib_name.ToCString());
51355137
}
5138+
cls.EnsureDeclarationLoaded();
51365139
CHECK_ERROR_HANDLE(cls.VerifyEntryPoint());
51375140
if (cls.NumTypeArguments() == 0) {
51385141
if (number_of_type_arguments != 0) {

runtime/vm/flag_list.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,12 @@ constexpr bool kDartPrecompiledRuntime = true;
3232
constexpr bool kDartPrecompiledRuntime = false;
3333
#endif
3434

35+
#if defined(DART_USE_BYTECODE)
36+
constexpr bool kDartUseBytecode = true;
37+
#else
38+
constexpr bool kDartUseBytecode = false;
39+
#endif
40+
3541
// List of all flags in the VM.
3642
// Flags can be one of three categories:
3743
// * P roduct flags: Can be set in any of the deployment modes, including in
@@ -191,7 +197,7 @@ constexpr bool kDartPrecompiledRuntime = false;
191197
D(trace_zones, bool, false, "Traces allocation sizes in the zone.") \
192198
P(truncating_left_shift, bool, true, \
193199
"Optimize left shift to truncate if possible") \
194-
P(use_bytecode_compiler, bool, false, "Compile from bytecode") \
200+
P(use_bytecode_compiler, bool, kDartUseBytecode, "Compile from bytecode") \
195201
P(use_compactor, bool, false, "Compact the heap during old-space GC.") \
196202
P(use_cha_deopt, bool, true, \
197203
"Use class hierarchy analysis even if it can cause deoptimization.") \

0 commit comments

Comments
 (0)