[vm/kernel] Fix bugs with expression evaluation in debug mode.
Fixes #33170: We had an assert that type variables are not bounded by "dynamic".
This assert was previously valid because the FE replaces "extends dynamic" with
"extends Object". However, it's not necessary in any way, and it's difficult to
access the Object type from outside the FE when synthesizing type parameters for
expression compilation.
Fixes #33171: The synthetic class generated during serialization to hold the
synthetic method may need to extend core library classes if the expression's
scope was within a core library class. In this case, the class finalizer needs
to avoid applying the restrictions on class extension to the synthetic class.
To facilitate this, we now use a single class name for all synthetic classes,
rather than copying the original class name. The name of the synthetic class has
no function at all, since the synthetic method is reparented to the original
class before execution.
Fixes #33239: The arguments descriptor passed to InvokeFunction for expression
functions which capture generic type parameters was using the wrong argument
count.
Fixes #33270: Use LookupClassAllowPrivate instead of LookupClass when resolving
the expression's scope in the VM.
Test Plan:
#33170: python tools/test.py -m debug -c dartk --strong service/evaluate_function_type_parameters_test
#33171: python tools/test.py -m debug -c dartk --strong service/eval_test
#33239: Updated "evaluate_function_type_parameters_test.dart".
#33270: Updated "eval_test.dart".
Change-Id: I376926bddd2224ec2322b43685d6c13831f1a8e7
Reviewed-on: https://dart-review.googlesource.com/56060
Commit-Queue: Samir Jindel <sjindel@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
diff --git a/pkg/front_end/lib/src/fasta/kernel/utils.dart b/pkg/front_end/lib/src/fasta/kernel/utils.dart
index d95f852..c799f7e 100644
--- a/pkg/front_end/lib/src/fasta/kernel/utils.dart
+++ b/pkg/front_end/lib/src/fasta/kernel/utils.dart
@@ -61,6 +61,8 @@
return byteSink.builder.takeBytes();
}
+const String kDebugClassName = "#DebugClass";
+
List<int> serializeProcedure(Procedure procedure) {
Library fakeLibrary =
new Library(new Uri(scheme: 'evaluate', path: 'source'));
@@ -70,7 +72,7 @@
CloneVisitor cloner = new CloneVisitor();
- Class fakeClass = new Class(name: realClass.name);
+ Class fakeClass = new Class(name: kDebugClassName);
for (TypeParameter typeParam in realClass.typeParameters) {
fakeClass.typeParameters.add(typeParam.accept(cloner));
}
diff --git a/runtime/observatory/tests/service/eval_test.dart b/runtime/observatory/tests/service/eval_test.dart
index 7753d04..8a7ff74 100644
--- a/runtime/observatory/tests/service/eval_test.dart
+++ b/runtime/observatory/tests/service/eval_test.dart
@@ -20,11 +20,18 @@
}
}
+class _MyClass {
+ void foo() {
+ debugger();
+ }
+}
+
void testFunction() {
int i = 0;
while (true) {
if (++i % 100000000 == 0) {
MyClass.method(10000);
+ (new _MyClass()).foo();
}
}
}
@@ -67,6 +74,23 @@
result = await instance.evaluate('this + frog');
expect(result.type, equals('Error'));
},
+ resumeIsolate,
+ hasStoppedAtBreakpoint,
+ (Isolate isolate) async {
+ ServiceMap stack = await isolate.getStack();
+
+ // Make sure we are in the right place.
+ expect(stack.type, equals('Stack'));
+ expect(stack['frames'].length, greaterThanOrEqualTo(2));
+ expect(stack['frames'][0].function.name, equals('foo'));
+ expect(stack['frames'][0].function.dartOwner.name, equals('_MyClass'));
+
+ var cls = stack['frames'][0].function.dartOwner;
+
+ dynamic result = await cls.evaluate("1+1");
+ print(result);
+ expect(result.valueAsString, equals("2"));
+ }
];
main(args) => runIsolateTests(args, tests, testeeConcurrent: testFunction);
diff --git a/runtime/observatory/tests/service/evaluate_function_type_parameters_test.dart b/runtime/observatory/tests/service/evaluate_function_type_parameters_test.dart
index 1ee01db..998f4bb 100644
--- a/runtime/observatory/tests/service/evaluate_function_type_parameters_test.dart
+++ b/runtime/observatory/tests/service/evaluate_function_type_parameters_test.dart
@@ -11,11 +11,11 @@
topLevel<S>() {
debugger();
- void inner1<T>() {
+ void inner1<T>(T x) {
debugger();
}
- inner1<int>();
+ inner1<int>(3);
void inner2() {
debugger();
@@ -64,6 +64,10 @@
result = await isolate.evalFrame(topFrame, "S.toString()");
print(result);
expect(result.valueAsString, equals("String"));
+
+ result = await isolate.evalFrame(topFrame, "x");
+ print(result);
+ expect(result.valueAsString, equals("3"));
},
resumeIsolate,
hasStoppedAtBreakpoint,
diff --git a/runtime/vm/class_finalizer.cc b/runtime/vm/class_finalizer.cc
index 3d4eea8..92ba3ed 100644
--- a/runtime/vm/class_finalizer.cc
+++ b/runtime/vm/class_finalizer.cc
@@ -3223,9 +3223,11 @@
cls.set_super_type(super_type);
}
- // If cls belongs to core lib, restrictions about allowed interfaces
- // are lifted.
- const bool cls_belongs_to_core_lib = cls.library() == Library::CoreLibrary();
+ // If cls belongs to core lib or is a synthetic class which could belong to
+ // the core library, the restrictions about allowed interfaces are lifted.
+ const bool exempt_from_hierarchy_restrictions =
+ cls.library() == Library::CoreLibrary() ||
+ String::Handle(cls.Name()).Equals(Symbols::DebugClassName());
// Resolve and check the super type and interfaces of cls.
visited->Add(cls_index);
@@ -3256,7 +3258,7 @@
// If cls belongs to core lib or to core lib's implementation, restrictions
// about allowed interfaces are lifted.
- if (!cls_belongs_to_core_lib) {
+ if (!exempt_from_hierarchy_restrictions) {
// Prevent extending core implementation classes.
bool is_error = false;
switch (interface_class.id()) {
@@ -3335,7 +3337,7 @@
}
// Verify that unless cls belongs to core lib, it cannot extend, implement,
// or mixin any of Null, bool, num, int, double, String, dynamic.
- if (!cls_belongs_to_core_lib) {
+ if (!exempt_from_hierarchy_restrictions) {
if (interface.IsBoolType() || interface.IsNullType() ||
interface.IsNumberType() || interface.IsIntType() ||
interface.IsDoubleType() || interface.IsStringType() ||
diff --git a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc
index fc17971..3b72081 100644
--- a/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc
+++ b/runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc
@@ -5955,9 +5955,8 @@
bound = forwarding_param.bound();
}
- if (I->strong() && !bound.IsObjectType() &&
+ if (I->strong() && !bound.IsObjectType() && !bound.IsDynamicType() &&
(I->reify_generic_functions() || dart_function.IsFactory())) {
- ASSERT(!bound.IsDynamicType());
TypeParameter& param = TypeParameter::Handle(Z);
if (dart_function.IsFactory()) {
param ^= TypeArguments::Handle(
diff --git a/runtime/vm/dart_entry.cc b/runtime/vm/dart_entry.cc
index 15ab7b8..2d50aad 100644
--- a/runtime/vm/dart_entry.cc
+++ b/runtime/vm/dart_entry.cc
@@ -438,7 +438,7 @@
descriptor.SetAt(kCountIndex, arg_count);
// Set number of positional arguments.
- descriptor.SetAt(kPositionalCountIndex, Smi::Handle(Smi::New(num_arguments)));
+ descriptor.SetAt(kPositionalCountIndex, arg_count);
// Set terminating null.
descriptor.SetAt((descriptor_len - 1), Object::null_object());
diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc
index 7457318..8be76ae 100644
--- a/runtime/vm/object.cc
+++ b/runtime/vm/object.cc
@@ -11551,7 +11551,8 @@
// Load the program with the debug procedure as a regular, independent
// program.
kernel::KernelLoader loader(kernel_pgm);
- loader.LoadProgram();
+ const Object& result = Object::Handle(loader.LoadProgram());
+ if (result.IsError()) return result.raw();
ASSERT(I->class_table()->NumCids() > num_cids &&
GrowableObjectArray::Handle(I->object_store()->libraries()).Length() ==
num_libs + 1);
@@ -11565,7 +11566,7 @@
String::New(Symbols::Symbol(Symbols::kDebugProcedureNameId)));
Class& fake_class = Class::Handle();
if (!klass.IsNull()) {
- fake_class = loaded.LookupClass(String::Handle(String::New(klass)));
+ fake_class = loaded.LookupClass(Symbols::DebugClassName());
ASSERT(!fake_class.IsNull());
callee = fake_class.LookupFunctionAllowPrivate(debug_name);
} else {
@@ -11587,7 +11588,7 @@
ASSERT(!real_library.IsNull());
Class& real_class = Class::Handle();
if (!klass.IsNull()) {
- real_class = real_library.LookupClass(String::Handle(String::New(klass)));
+ real_class = real_library.LookupClassAllowPrivate(klass);
} else {
real_class = real_library.toplevel_class();
}
@@ -11620,7 +11621,7 @@
real_arguments.SetAt(i + 1, arg);
}
const Array& args_desc = Array::Handle(
- ArgumentsDescriptor::New(num_type_args, real_arguments.Length()));
+ ArgumentsDescriptor::New(num_type_args, arguments.Length()));
return DartEntry::InvokeFunction(callee, real_arguments, args_desc);
#endif
}
diff --git a/runtime/vm/symbols.h b/runtime/vm/symbols.h
index f220ca3..4613aff 100644
--- a/runtime/vm/symbols.h
+++ b/runtime/vm/symbols.h
@@ -457,6 +457,7 @@
V(_AsyncStarListenHelper, "_asyncStarListenHelper") \
V(GrowRegExpStack, "_growRegExpStack") \
V(DebugProcedureName, ":Eval") \
+ V(DebugClassName, "#DebugClass") \
V(vm_entry_point, "vm.entry_point")
// Contains a list of frequently used strings in a canonicalized form. This
