[vm/kernel] Index all source and line starts before using them
By indexing all sources in a concatenated dill file and then using that
index to find the sources and line starts we can fix missing information
(leading to crashes when collecting coverage) in for instance circular
instances.
Change-Id: I454cff57f84f21de72f6486e1cecf15d1389a498
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/96901
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Vyacheslav Egorov <vegorov@google.com>
diff --git a/pkg/vm/test/incremental_compiler_test.dart b/pkg/vm/test/incremental_compiler_test.dart
index 6e0fa6f..a280c93 100644
--- a/pkg/vm/test/incremental_compiler_test.dart
+++ b/pkg/vm/test/incremental_compiler_test.dart
@@ -437,7 +437,7 @@
});
await portLineCompleter.future;
print("Compiler terminated with ${await vm.exitCode} exit code");
- }, skip: "Currently fails");
+ });
});
group('reload', () {
diff --git a/runtime/vm/bootstrap.cc b/runtime/vm/bootstrap.cc
index fc59b64..8c05417 100644
--- a/runtime/vm/bootstrap.cc
+++ b/runtime/vm/bootstrap.cc
@@ -89,7 +89,7 @@
const String& msg = String::Handle(String::New(message_buffer, Heap::kOld));
return ApiError::New(msg, Heap::kOld);
}
- kernel::KernelLoader loader(program);
+ kernel::KernelLoader loader(program, /*uri_to_source_table=*/nullptr);
Isolate* isolate = thread->isolate();
diff --git a/runtime/vm/kernel_loader.cc b/runtime/vm/kernel_loader.cc
index 1a2f00a..acfd816 100644
--- a/runtime/vm/kernel_loader.cc
+++ b/runtime/vm/kernel_loader.cc
@@ -175,7 +175,10 @@
class_offset + class_size - 4 - (procedure_count_ + 1) * 4;
}
-KernelLoader::KernelLoader(Program* program)
+using UriToSourceTable = DirectChainedHashMap<UriToSourceTableTrait>;
+
+KernelLoader::KernelLoader(Program* program,
+ UriToSourceTable* uri_to_source_table)
: program_(program),
thread_(Thread::Current()),
zone_(thread_->zone()),
@@ -209,7 +212,7 @@
"Trying to load a concatenated dill file at a time where that is "
"not allowed");
}
- InitializeFields();
+ InitializeFields(uri_to_source_table);
}
void KernelLoader::ReadObfuscationProhibitions() {
@@ -223,7 +226,7 @@
TIMELINE_DURATION(thread, Isolate, "LoadKernel");
if (program->is_single_program()) {
- KernelLoader loader(program);
+ KernelLoader loader(program, /*uri_to_source_table=*/nullptr);
return Object::Handle(loader.LoadProgram(process_pending_classes));
}
@@ -233,8 +236,50 @@
Zone* zone = thread->zone();
Library& library = Library::Handle(zone);
- // Create "fake programs" for each sub-program.
intptr_t subprogram_count = subprogram_file_starts.length() - 1;
+
+ // First index all source tables.
+ UriToSourceTable uri_to_source_table;
+ UriToSourceTableEntry wrapper;
+ for (intptr_t i = subprogram_count - 1; i >= 0; --i) {
+ intptr_t subprogram_start = subprogram_file_starts.At(i);
+ intptr_t subprogram_end = subprogram_file_starts.At(i + 1);
+ Thread* thread_ = Thread::Current();
+ Zone* zone_ = thread_->zone();
+ TranslationHelper translation_helper(thread);
+ KernelReaderHelper helper_(zone_, &translation_helper,
+ program->kernel_data() + subprogram_start,
+ subprogram_end - subprogram_start, 0);
+ const intptr_t source_table_size = helper_.SourceTableSize();
+ for (intptr_t index = 0; index < source_table_size; ++index) {
+ const String& uri_string = helper_.SourceTableUriFor(index);
+ wrapper.uri = &uri_string;
+ TypedData& line_starts =
+ TypedData::Handle(Z, helper_.GetLineStartsFor(index));
+ if (line_starts.Length() == 0) continue;
+ const String& script_source = helper_.GetSourceFor(index);
+ wrapper.uri = &uri_string;
+ UriToSourceTableEntry* pair = uri_to_source_table.LookupValue(&wrapper);
+ if (pair != NULL) {
+ // At least two entries with content. Unless the content is the same
+ // that's not valid.
+ if (pair->sources->CompareTo(script_source) != 0 ||
+ !pair->line_starts->OperatorEquals(line_starts)) {
+ FATAL(
+ "Invalid kernel binary: Contains at least two source entries "
+ "that do not agree.");
+ }
+ } else {
+ UriToSourceTableEntry* tmp = new UriToSourceTableEntry();
+ tmp->uri = &uri_string;
+ tmp->sources = &script_source;
+ tmp->line_starts = &line_starts;
+ uri_to_source_table.Insert(tmp);
+ }
+ }
+ }
+
+ // Create "fake programs" for each sub-program.
for (intptr_t i = subprogram_count - 1; i >= 0; --i) {
intptr_t subprogram_start = subprogram_file_starts.At(i);
intptr_t subprogram_end = subprogram_file_starts.At(i + 1);
@@ -243,7 +288,7 @@
reader.set_offset(0);
Program* subprogram = Program::ReadFrom(&reader);
ASSERT(subprogram->is_single_program());
- KernelLoader loader(subprogram);
+ KernelLoader loader(subprogram, &uri_to_source_table);
Object& load_result = Object::Handle(loader.LoadProgram(false));
if (load_result.IsError()) return load_result;
@@ -301,7 +346,7 @@
return String::null();
}
-void KernelLoader::InitializeFields() {
+void KernelLoader::InitializeFields(UriToSourceTable* uri_to_source_table) {
const intptr_t source_table_size = helper_.SourceTableSize();
const Array& scripts =
Array::Handle(Z, Array::New(source_table_size, Heap::kOld));
@@ -370,7 +415,7 @@
Script& script = Script::Handle(Z);
for (intptr_t index = 0; index < source_table_size; ++index) {
- script = LoadScriptAt(index);
+ script = LoadScriptAt(index, uri_to_source_table);
scripts.SetAt(index, script);
}
@@ -810,7 +855,7 @@
// kernel files, these will constitute the modified libraries.
*is_empty_program = true;
if (program->is_single_program()) {
- KernelLoader loader(program);
+ KernelLoader loader(program, /*uri_to_source_table=*/nullptr);
loader.walk_incremental_kernel(modified_libs, is_empty_program,
p_num_classes, p_num_procedures);
}
@@ -828,7 +873,7 @@
reader.set_offset(0);
Program* subprogram = Program::ReadFrom(&reader);
ASSERT(subprogram->is_single_program());
- KernelLoader loader(subprogram);
+ KernelLoader loader(subprogram, /*uri_to_source_table=*/nullptr);
loader.walk_incremental_kernel(modified_libs, is_empty_program,
p_num_classes, p_num_procedures);
delete subprogram;
@@ -1948,33 +1993,49 @@
return klass;
}
-RawScript* KernelLoader::LoadScriptAt(intptr_t index) {
+RawScript* KernelLoader::LoadScriptAt(intptr_t index,
+ UriToSourceTable* uri_to_source_table) {
const String& uri_string = helper_.SourceTableUriFor(index);
const String& import_uri_string =
helper_.SourceTableImportUriFor(index, program_->binary_version());
- const String& script_source = helper_.GetSourceFor(index);
+
String& sources = String::Handle(Z);
- TypedData& line_starts =
- TypedData::Handle(Z, helper_.GetLineStartsFor(index));
- if (script_source.raw() == Symbols::Empty().raw() &&
- line_starts.Length() == 0 && uri_string.Length() > 0) {
- // Entry included only to provide URI - actual source should already exist
- // in the VM, so try to find it.
- Library& lib = Library::Handle(Z);
- Script& script = Script::Handle(Z);
- const GrowableObjectArray& libs =
- GrowableObjectArray::Handle(isolate_->object_store()->libraries());
- for (intptr_t i = 0; i < libs.Length(); i++) {
- lib ^= libs.At(i);
- script = lib.LookupScript(uri_string, /* useResolvedUri = */ true);
- if (!script.IsNull() && script.kind() == RawScript::kKernelTag) {
- sources ^= script.Source();
- line_starts ^= script.line_starts();
- break;
- }
+ TypedData& line_starts = TypedData::Handle(Z);
+
+ if (uri_to_source_table != nullptr) {
+ UriToSourceTableEntry wrapper;
+ wrapper.uri = &uri_string;
+ UriToSourceTableEntry* pair = uri_to_source_table->LookupValue(&wrapper);
+ if (pair != nullptr) {
+ sources ^= pair->sources->raw();
+ line_starts ^= pair->line_starts->raw();
}
- } else {
- sources = script_source.raw();
+ }
+
+ if (sources.IsNull() || line_starts.IsNull()) {
+ const String& script_source = helper_.GetSourceFor(index);
+ line_starts ^= helper_.GetLineStartsFor(index);
+
+ if (script_source.raw() == Symbols::Empty().raw() &&
+ line_starts.Length() == 0 && uri_string.Length() > 0) {
+ // Entry included only to provide URI - actual source should already exist
+ // in the VM, so try to find it.
+ Library& lib = Library::Handle(Z);
+ Script& script = Script::Handle(Z);
+ const GrowableObjectArray& libs =
+ GrowableObjectArray::Handle(isolate_->object_store()->libraries());
+ for (intptr_t i = 0; i < libs.Length(); i++) {
+ lib ^= libs.At(i);
+ script = lib.LookupScript(uri_string, /* useResolvedUri = */ true);
+ if (!script.IsNull() && script.kind() == RawScript::kKernelTag) {
+ sources ^= script.Source();
+ line_starts ^= script.line_starts();
+ break;
+ }
+ }
+ } else {
+ sources = script_source.raw();
+ }
}
const Script& script =
diff --git a/runtime/vm/kernel_loader.h b/runtime/vm/kernel_loader.h
index 9b176ff..9f339bc 100644
--- a/runtime/vm/kernel_loader.h
+++ b/runtime/vm/kernel_loader.h
@@ -147,9 +147,36 @@
DISALLOW_COPY_AND_ASSIGN(ClassIndex);
};
+struct UriToSourceTableEntry : public ZoneAllocated {
+ UriToSourceTableEntry() {}
+
+ const String* uri = nullptr;
+ const String* sources = nullptr;
+ const TypedData* line_starts = nullptr;
+};
+
+struct UriToSourceTableTrait {
+ typedef UriToSourceTableEntry* Value;
+ typedef const UriToSourceTableEntry* Key;
+ typedef UriToSourceTableEntry* Pair;
+
+ static Key KeyOf(Pair kv) { return kv; }
+
+ static Value ValueOf(Pair kv) { return kv; }
+
+ static inline intptr_t Hashcode(Key key) { return key->uri->Hash(); }
+
+ static inline bool IsKeyEqual(Pair kv, Key key) {
+ // Only compare uri.
+ return kv->uri->CompareTo(*key->uri) == 0;
+ }
+};
+
class KernelLoader : public ValueObject {
public:
- explicit KernelLoader(Program* program);
+ explicit KernelLoader(
+ Program* program,
+ DirectChainedHashMap<UriToSourceTableTrait>* uri_to_source_table);
static Object& LoadEntireProgram(Program* program,
bool process_pending_classes = true);
@@ -252,7 +279,8 @@
const ExternalTypedData& kernel_data,
intptr_t data_program_offset);
- void InitializeFields();
+ void InitializeFields(
+ DirectChainedHashMap<UriToSourceTableTrait>* uri_to_source_table);
static void index_programs(kernel::Reader* reader,
GrowableArray<intptr_t>* subprogram_file_starts);
void walk_incremental_kernel(BitVector* modified_libs,
@@ -288,7 +316,9 @@
RawArray* MakeFieldsArray();
RawArray* MakeFunctionsArray();
- RawScript* LoadScriptAt(intptr_t index);
+ RawScript* LoadScriptAt(
+ intptr_t index,
+ DirectChainedHashMap<UriToSourceTableTrait>* uri_to_source_table);
// If klass's script is not the script at the uri index, return a PatchClass
// for klass whose script corresponds to the uri index.
diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc
index f612702..84718f5 100644
--- a/runtime/vm/object.cc
+++ b/runtime/vm/object.cc
@@ -11104,7 +11104,7 @@
String::New("Kernel isolate returned ill-formed kernel.")));
}
- kernel::KernelLoader loader(kernel_pgm);
+ kernel::KernelLoader loader(kernel_pgm, /*uri_to_source_table=*/nullptr);
const Object& result = Object::Handle(
loader.LoadExpressionEvaluationFunction(library_url, klass));
