From 587e6308bf6866f7ffa4e5b83dce03ea266d45a4 Mon Sep 17 00:00:00 2001 From: Martin Kustermann Date: Fri, 4 Aug 2023 11:07:46 +0000 Subject: [PATCH] [vm] Fix expression evaluation in context of closures When evaluating an expressino at a breakpoint that's inside a closure, the closure may refer to anything it closes over. That includes the `this` from an enclosing method. So depending on whether a closure's parent chain is an instance method or a static method, the expression evaluation function is going to be an instance method or a static method. => We walk up the parent chain to determine the correct enclosing class. => This avoids making a Type object with a cid from a top-level class (see [0]) Handling this correctly will now try to get the `this` when invoking an eval function that has an enclosing instance method. Though we may often have "" the `this` (e.g. due to not capturing it in closure context chain). => We still allow running the expression evaluation function in this case, but only if the expression being evaluated doesn't access `this`. A similar issue occurs when trying to use variables in the eval expression that the closure didn't capture. This results in a confusing CFE compile-time error. This is a separate issue and tracked in [1]. => We update the test to distinuish the cases that this CL makes passing and those that are failing due to [1]. Fixes [0] https://github.com/dart-lang/sdk/issues/53061 See also [1] https://github.com/dart-lang/sdk/issues/53087 TEST=Fixes part of service/evaluate_activation_test failures Change-Id: I3bb24e7338c7b2f12d5340311d944cb59a455641 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/317540 Reviewed-by: Alexander Markov Reviewed-by: Slava Egorov Commit-Queue: Martin Kustermann --- pkg/vm/lib/target/vm.dart | 1 + pkg/vm_service/test/common/test_helper.dart | 19 +- .../test/evaluate_inside_closures_test.dart | 3 + .../service/evaluate_activation_test.dart | 103 +++++-- ..._optimized_out_receiver_on_stack_test.dart | 125 +++++++++ runtime/vm/compiler/frontend/scope_builder.cc | 2 + runtime/vm/debugger.cc | 41 ++- runtime/vm/object.cc | 251 ++++++++++-------- runtime/vm/object.h | 22 +- runtime/vm/parser.h | 8 + runtime/vm/scopes.cc | 2 +- runtime/vm/scopes.h | 58 ++-- runtime/vm/service.cc | 45 ++-- 13 files changed, 475 insertions(+), 205 deletions(-) create mode 100644 runtime/observatory/tests/service/evaluate_expression_with_optimized_out_receiver_on_stack_test.dart diff --git a/pkg/vm/lib/target/vm.dart b/pkg/vm/lib/target/vm.dart index b06bdd0a715..73e510f8960 100644 --- a/pkg/vm/lib/target/vm.dart +++ b/pkg/vm/lib/target/vm.dart @@ -398,6 +398,7 @@ class VmTarget extends Target { // purposes. bool allowPlatformPrivateLibraryAccess(Uri importer, Uri imported) => super.allowPlatformPrivateLibraryAccess(importer, imported) || + importer.path.contains('runtime/observatory/tests') || importer.path.contains('runtime/tests/vm/dart') || importer.path.contains('tests/standalone/io') || importer.path.contains('test-lib') || diff --git a/pkg/vm_service/test/common/test_helper.dart b/pkg/vm_service/test/common/test_helper.dart index c6f1a590534..5eb20ee02c3 100644 --- a/pkg/vm_service/test/common/test_helper.dart +++ b/pkg/vm_service/test/common/test_helper.dart @@ -93,11 +93,15 @@ class _ServiceTesteeRunner { class _ServiceTesteeLauncher { io.Process? process; List args; + bool killedByTester = false; + final _exitCodeCompleter = Completer(); _ServiceTesteeLauncher(String script) : args = [_getTestUri(script).toFilePath()]; + Future get exitCode => _exitCodeCompleter.future; + // Spawn the testee process. Future _spawnProcess( bool pause_on_start, @@ -223,12 +227,7 @@ class _ServiceTesteeLauncher { .listen((line) { io.stdout.write('>testee>err> ${line}\n'); }); - process!.exitCode.then((exitCode) { - if ((exitCode != 0) && !killedByTester) { - throw "Testee exited with $exitCode"; - } - print("** Process exited: $exitCode"); - }); + process!.exitCode.then(_exitCodeCompleter.complete); return completer.future; }); } @@ -320,6 +319,14 @@ class _ServiceTesterRunner { print('All service tests completed successfully.'); process.requestExit(); }); + + final exitCode = await process.exitCode; + if (exitCode != 0) { + if (!process.killedByTester) { + throw "Testee exited with unexpected exitCode: $exitCode"; + } + } + print("** Process exited: $exitCode"); } Future getFirstIsolate(VmService service) async { diff --git a/pkg/vm_service/test/evaluate_inside_closures_test.dart b/pkg/vm_service/test/evaluate_inside_closures_test.dart index 434775a8fb3..9571cdb7e5d 100644 --- a/pkg/vm_service/test/evaluate_inside_closures_test.dart +++ b/pkg/vm_service/test/evaluate_inside_closures_test.dart @@ -25,10 +25,13 @@ class C { void instanceMethod() { ((int y) { debugger(); + use(this); })(78); } } +void use(_) {} + testMain() { C c = C(); C.staticMethod(); diff --git a/runtime/observatory/tests/service/evaluate_activation_test.dart b/runtime/observatory/tests/service/evaluate_activation_test.dart index 53a75b6ba6e..28dab1aacd4 100644 --- a/runtime/observatory/tests/service/evaluate_activation_test.dart +++ b/runtime/observatory/tests/service/evaluate_activation_test.dart @@ -65,6 +65,26 @@ class C { }; }); } + + method5(methodParam) { + var methodTemp = 4; + use(methodTemp); + [5].forEach((outerParam) { + var outerTemp = 6; + use(outerTemp); + closureWithReturnedHome = (innerParam) { + use(this); + use(methodParam); + use(methodTemp); + use(outerParam); + use(outerTemp); + use(innerParam); + var innerTemp = 8; + use(innerTemp); + breakHere(); + }; + }); + } } Future testMethod(Isolate isolate) async { @@ -85,20 +105,20 @@ Future testMethod(Isolate isolate) async { dynamic frameNumber = 1, r; r = await isolate.evalFrame(frameNumber, '123'); //# instance: ok expect(r.valueAsString, equals('123')); //# instance: continued - r = await isolate.evalFrame(frameNumber, 'this'); //# scope: ok - expect(r.clazz.name, equals('C')); //# scope: continued + r = await isolate.evalFrame(frameNumber, 'this'); //# instance: continued + expect(r.valueAsString, equals('')); //# instance: continued r = await isolate.evalFrame(frameNumber, 'instVar'); //# instance: continued - expect(r.valueAsString, equals('1')); //# instance: continued + expect(r.valueAsString, equals('')); //# instance: continued r = await isolate.evalFrame(frameNumber, 'classVar'); //# instance: continued expect(r.valueAsString, equals('2')); //# instance: continued - r = await isolate.evalFrame(frameNumber, 'methodParam'); //# scope: continued - expect(r.valueAsString, equals('3')); //# scope: continued + r = await isolate.evalFrame(frameNumber, 'methodParam'); //# scope: ok + expect(r.valueAsString, equals('')); //# scope: continued r = await isolate.evalFrame(frameNumber, 'methodTemp'); //# scope: continued - expect(r.valueAsString, equals('4')); //# scope: continued + expect(r.valueAsString, equals('')); //# scope: continued r = await isolate.evalFrame(frameNumber, 'outerParam'); //# scope: continued - expect(r.valueAsString, equals('5')); //# scope: continued + expect(r.valueAsString, equals('')); //# scope: continued r = await isolate.evalFrame(frameNumber, 'outerTemp'); //# scope: continued - expect(r.valueAsString, equals('6')); //# scope: continued + expect(r.valueAsString, equals('')); //# scope: continued r = await isolate.evalFrame(frameNumber, 'innerParam'); //# instance: continued expect(r.valueAsString, equals('7')); //# instance: continued r = await isolate.evalFrame(frameNumber, 'innerTemp'); //# instance: continued @@ -137,13 +157,13 @@ Future testMethod2(Isolate isolate) async { r = await isolate.evalFrame(frameNumber, 'classVar'); expect(r.valueAsString, equals('2')); r = await isolate.evalFrame(frameNumber, 'methodParam'); - expect(r.valueAsString, equals('3')); //# scope: continued - r = await isolate.evalFrame(frameNumber, 'methodTemp'); - expect(r.valueAsString, equals('4')); //# scope: continued - r = await isolate.evalFrame(frameNumber, 'outerParam'); - expect(r.valueAsString, equals('5')); //# scope: continued - r = await isolate.evalFrame(frameNumber, 'outerTemp'); - expect(r.valueAsString, equals('6')); //# scope: continued + expect(r.valueAsString, equals('')); //# scope: continued + r = await isolate.evalFrame(frameNumber, 'methodTemp'); //# scope: continued + expect(r.valueAsString, equals('')); //# scope: continued + r = await isolate.evalFrame(frameNumber, 'outerParam'); //# scope: continued + expect(r.valueAsString, equals('')); //# scope: continued + r = await isolate.evalFrame(frameNumber, 'outerTemp'); //# scope: continued + expect(r.valueAsString, equals('')); //# scope: continued r = await isolate.evalFrame(frameNumber, 'innerParam'); expect(r.valueAsString, equals('7')); r = await isolate.evalFrame(frameNumber, 'innerTemp'); @@ -213,10 +233,10 @@ Future testMethod4(Isolate isolate) async { dynamic frameNumber = 1, r; r = await isolate.evalFrame(frameNumber, '123'); //# instance: continued expect(r.valueAsString, equals('123')); //# instance: continued - r = await isolate.evalFrame(frameNumber, 'this'); //# scope: continued - expect(r.clazz.name, equals('C')); //# scope: continued + r = await isolate.evalFrame(frameNumber, 'this'); //# instance: continued + expect(r.valueAsString, equals('')); //# instance: continued r = await isolate.evalFrame(frameNumber, 'instVar'); //# instance: continued - expect(r.valueAsString, equals('1')); //# instance: continued + expect(r.valueAsString, equals('')); //# instance: continued r = await isolate.evalFrame(frameNumber, 'classVar'); //# instance: continued expect(r.valueAsString, equals('2')); //# instance: continued r = await isolate.evalFrame(frameNumber, 'methodParam'); //# scope: continued @@ -243,11 +263,58 @@ Future testMethod4(Isolate isolate) async { expect(hitBreakpoint, isTrue); } +Future testMethod5(Isolate isolate) async { + Library rootLib = await isolate.rootLibrary.load() as Library; + ServiceFunction function = + rootLib.functions.singleWhere((f) => f.name == 'breakHere'); + Breakpoint bpt = await isolate.addBreakpointAtEntry(function); + print("Breakpoint: $bpt"); + + bool hitBreakpoint = false; + var stream = await isolate.vm.getEventStream(VM.kDebugStream); + stream.firstWhere((event) { + print("Event $event"); + return event.kind == ServiceEvent.kPauseBreakpoint; + }).then((event) async { + dynamic frameNumber = 1, r; + r = await isolate.evalFrame(frameNumber, '123'); //# instance: continued + expect(r.valueAsString, equals('123')); //# instance: continued + r = await isolate.evalFrame(frameNumber, 'this'); //# instance: continued + expect(r.clazz.name, equals('C')); //# instance: continued + r = await isolate.evalFrame(frameNumber, 'instVar'); //# instance: continued + expect(r.valueAsString, equals('1')); //# instance: continued + r = await isolate.evalFrame(frameNumber, 'classVar'); //# instance: continued + expect(r.valueAsString, equals('2')); //# instance: continued + r = await isolate.evalFrame(frameNumber, 'methodParam'); //# instance: continued + expect(r.valueAsString, equals('3')); //# instance: continued + r = await isolate.evalFrame(frameNumber, 'methodTemp'); //# instance: continued + expect(r.valueAsString, equals('4')); //# instance: continued + r = await isolate.evalFrame(frameNumber, 'outerParam'); //# instance: continued + expect(r.valueAsString, equals('5')); //# instance: continued + r = await isolate.evalFrame(frameNumber, 'outerTemp'); //# instance: continued + expect(r.valueAsString, equals('6')); //# instance: continued + r = await isolate.evalFrame(frameNumber, 'innerParam'); //# instance: continued + expect(r.valueAsString, equals('7')); //# instance: continued + r = await isolate.evalFrame(frameNumber, 'innerTemp'); //# instance: continued + expect(r.valueAsString, equals('8')); //# instance: continued + r = await isolate.evalFrame(frameNumber, 'math.sqrt'); //# instance: continued + expect(r.isClosure, isTrue); //# instance: continued + + hitBreakpoint = true; + }).whenComplete(isolate.resume); + + var result = await rootLib.evaluate( + '(){ new C().method5(3); C.closureWithReturnedHome(7); }()'); + print("Result $result"); + expect(hitBreakpoint, isTrue); +} + var tests = [ testMethod, testMethod2, testMethod3, testMethod4, + testMethod5, ]; main(args) => runIsolateTests(args, tests); diff --git a/runtime/observatory/tests/service/evaluate_expression_with_optimized_out_receiver_on_stack_test.dart b/runtime/observatory/tests/service/evaluate_expression_with_optimized_out_receiver_on_stack_test.dart new file mode 100644 index 00000000000..1c64e235a23 --- /dev/null +++ b/runtime/observatory/tests/service/evaluate_expression_with_optimized_out_receiver_on_stack_test.dart @@ -0,0 +1,125 @@ +// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +// VMOptions=--deterministic + +import 'dart:async'; +import 'dart:_internal'; // ignore: import_internal_library, unused_import + +import 'package:observatory/service_io.dart'; +import 'package:test/test.dart'; + +import 'test_helper.dart'; + +String examineStackExpression(String variableName) { + // The returned string is the evaluation expression. We try to make it so that + // the evaluated expression exercises OSR, Deopt, StackTrace. + // + // Even though the expression we build doesn't use `this`, the expression is + // evaluated in the context of a closure that's inside an instance method. So + // the evaluation function will be an instance method that does receive a + // receiver that was "". + // => The purpose of the test is to see if having the sentinal as receiver + // causes any issues. + final entries = [ + // Obtain stack while eval function is unoptimized + 'triggerStackTrace()', + + // Get eval function OSRed + 'for (int i = 0; i < 100 * 1000; ++i) i', + + // Obtain stack while eval function is optimized. + 'triggerStackTrace()', + + // Deopt eval function. + 'triggerDeopt()', + ]; + final round = entries.join(', '); + return 'returnFirst([$variableName, $round, $round, $round])'; +} + +@pragma('vm:never-inline') +dynamic triggerDeopt() { + print('triggerDeopt'); + VMInternalsForTesting.deoptimizeFunctionsOnStack(); // ignore: undefined_identifier +} + +@pragma('vm:never-inline') +dynamic triggerStackTrace() { + print('triggerStackTrace'); + return StackTrace.current; +} + +@pragma('vm:never-inline') +dynamic returnFirst(List l) { + print('returnFirst'); + return l.first; +} + +breakHere() {} + +use(dynamic v) => v; + +class C { + var instVar = 1; + + method(methodParam) { + var methodTemp = 3; + use(methodTemp); + [4].forEach((outerParam) { + var outerTemp = 5; + use(outerTemp); + [6].forEach((innerParam) { + var innerTemp = 7; + use(innerTemp); + breakHere(); + }); + }); + } +} + +Future testMethod(Isolate isolate) async { + final rootLib = await isolate.rootLibrary.load() as Library; + final function = rootLib.functions.singleWhere((f) => f.name == 'breakHere'); + final bpt = await isolate.addBreakpointAtEntry(function); + print("Breakpoint: $bpt"); + + final stream = await isolate.vm.getEventStream(VM.kDebugStream); + final hitBreakpointFuture = stream.firstWhere((event) { + print("Event $event"); + return event.kind == ServiceEvent.kPauseBreakpoint; + }); + + Future handleBreakpoint() async { + Future checkValue(String expr, String value) async { + print('Evaluating "$expr".'); + const frameNumber = 1; + final dynamic r = await isolate.evalFrame(frameNumber, expr); + print(' -> result $r'); + expect(r.valueAsString, equals(value)); + } + + print('waiting for breakpoint'); + await hitBreakpointFuture; + print('got breakpoint'); + await checkValue(examineStackExpression('this'), ''); + await checkValue(examineStackExpression('instVar'), ''); + await checkValue(examineStackExpression('innerParam'), '6'); + await checkValue(examineStackExpression('innerTemp'), '7'); + + await isolate.resume(); + print('resuming '); + } + + await Future.wait([ + handleBreakpoint(), + rootLib.evaluate('C().method(2)'), + ]); +} + +final tests = [ + testMethod, +]; + +main(args) => runIsolateTests(args, tests); diff --git a/runtime/vm/compiler/frontend/scope_builder.cc b/runtime/vm/compiler/frontend/scope_builder.cc index 65a9eb3f729..badc48ebc17 100644 --- a/runtime/vm/compiler/frontend/scope_builder.cc +++ b/runtime/vm/compiler/frontend/scope_builder.cc @@ -1928,6 +1928,8 @@ void ScopeBuilder::HandleLoadReceiver() { // use-site also includes the [receiver]. scope_->CaptureVariable(parsed_function_->receiver_var()); } + + parsed_function_->set_receiver_used(); } void ScopeBuilder::HandleSpecialLoad(LocalVariable** variable, diff --git a/runtime/vm/debugger.cc b/runtime/vm/debugger.cc index c5e55f2459e..fdc804a8274 100644 --- a/runtime/vm/debugger.cc +++ b/runtime/vm/debugger.cc @@ -1055,28 +1055,27 @@ ObjectPtr ActivationFrame::EvaluateCompiledExpression( const Array& type_definitions, const Array& arguments, const TypeArguments& type_arguments) { - if (function().IsClosureFunction()) { - return Library::Handle(Library()).EvaluateCompiledExpression( - kernel_buffer, type_definitions, arguments, type_arguments); - } else if (function().is_static()) { - const Class& cls = Class::Handle(function().Owner()); - return cls.EvaluateCompiledExpression(kernel_buffer, type_definitions, - arguments, type_arguments); - } else { - const Object& receiver = Object::Handle(GetReceiver()); - if (receiver.ptr() == Object::optimized_out().ptr()) { - // Cannot execute an instance method without a receiver. - return Object::optimized_out().ptr(); - } - const Class& method_cls = Class::Handle(function().Owner()); - ASSERT(receiver.IsInstance() || receiver.IsNull()); - if (!(receiver.IsInstance() || receiver.IsNull())) { - return Object::null(); - } - const Instance& inst = Instance::Cast(receiver); - return inst.EvaluateCompiledExpression( - method_cls, kernel_buffer, type_definitions, arguments, type_arguments); + auto thread = Thread::Current(); + auto zone = thread->zone(); + + // The expression evaluation function will get all it's captured state passed + // as parameters (with `this` being the exception). As a result, we treat the + // expression evaluation function as either a top-level, static or instance + // method. + const auto& outermost = + Function::Handle(zone, function().GetOutermostFunction()); + const auto& klass = Class::Handle(zone, outermost.Owner()); + const auto& library = Library::Handle(zone, klass.library()); + + auto& receiver = Object::Handle(zone); + if (!klass.IsTopLevel() && !outermost.is_static()) { + receiver = GetReceiver(); + RELEASE_ASSERT(receiver.IsInstance() || + receiver.ptr() == Object::optimized_out().ptr()); } + return Instance::EvaluateCompiledExpression(thread, receiver, library, klass, + kernel_buffer, type_definitions, + arguments, type_arguments); } TypeArgumentsPtr ActivationFrame::BuildParameters( diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc index 12a9ae05fe4..ffc33752514 100644 --- a/runtime/vm/object.cc +++ b/runtime/vm/object.cc @@ -4800,32 +4800,164 @@ ObjectPtr Class::Invoke(const String& function_name, return DartEntry::InvokeFunction(function, args, args_descriptor_array); } +#if !defined(DART_PRECOMPILED_RUNTIME) + +static ObjectPtr LoadExpressionEvaluationFunction( + Zone* zone, + const ExternalTypedData& kernel_buffer, + const String& library_url, + const String& klass) { + std::unique_ptr kernel_pgm = + kernel::Program::ReadFromTypedData(kernel_buffer); + + if (kernel_pgm == nullptr) { + return ApiError::New(String::Handle( + zone, String::New("Kernel isolate returned ill-formed kernel."))); + } + + auto& result = Object::Handle(zone); + { + kernel::KernelLoader loader(kernel_pgm.get(), + /*uri_to_source_table=*/nullptr); + result = loader.LoadExpressionEvaluationFunction(library_url, klass); + kernel_pgm.reset(); + } + if (result.IsError()) return result.ptr(); + return Function::Cast(result).ptr(); +} + +static bool EvaluationFunctionNeedsReceiver(Thread* thread, + Zone* zone, + const Function& eval_function) { + auto parsed_function = new ParsedFunction( + thread, Function::ZoneHandle(zone, eval_function.ptr())); + parsed_function->EnsureKernelScopes(); + return parsed_function->is_receiver_used(); +} + static ObjectPtr EvaluateCompiledExpressionHelper( + Zone* zone, + const Function& eval_function, + const Array& type_definitions, + const Array& arguments, + const TypeArguments& type_arguments) { + // type_arguments is null if all type arguments are dynamic. + if (type_definitions.Length() == 0 || type_arguments.IsNull()) { + return DartEntry::InvokeFunction(eval_function, arguments); + } + + intptr_t num_type_args = type_arguments.Length(); + const auto& real_arguments = + Array::Handle(zone, Array::New(arguments.Length() + 1)); + real_arguments.SetAt(0, type_arguments); + Object& arg = Object::Handle(zone); + for (intptr_t i = 0; i < arguments.Length(); ++i) { + arg = arguments.At(i); + real_arguments.SetAt(i + 1, arg); + } + + const Array& args_desc = + Array::Handle(zone, ArgumentsDescriptor::NewBoxed( + num_type_args, arguments.Length(), Heap::kNew)); + return DartEntry::InvokeFunction(eval_function, real_arguments, args_desc); +} + +#endif // !defined(DART_PRECOMPILED_RUNTIME) + +ObjectPtr Library::EvaluateCompiledExpression( const ExternalTypedData& kernel_buffer, const Array& type_definitions, - const String& library_url, - const String& klass, const Array& arguments, - const TypeArguments& type_arguments); + const TypeArguments& type_arguments) const { + const auto& klass = Class::Handle(toplevel_class()); + return klass.EvaluateCompiledExpression(kernel_buffer, type_definitions, + arguments, type_arguments); +} ObjectPtr Class::EvaluateCompiledExpression( const ExternalTypedData& kernel_buffer, const Array& type_definitions, const Array& arguments, const TypeArguments& type_arguments) const { - ASSERT(Thread::Current()->IsDartMutatorThread()); - if (IsInternalOnlyClassId(id()) || (id() == kTypeArgumentsCid)) { - const Instance& exception = Instance::Handle(String::New( - "Expressions can be evaluated only with regular Dart instances")); - const Instance& stacktrace = Instance::Handle(); - return UnhandledException::New(exception, stacktrace); + auto thread = Thread::Current(); + const auto& library = Library::Handle(thread->zone(), this->library()); + return Instance::EvaluateCompiledExpression( + thread, Instance::null_object(), library, *this, kernel_buffer, + type_definitions, arguments, type_arguments); +} + +ObjectPtr Instance::EvaluateCompiledExpression( + const Class& klass, + const ExternalTypedData& kernel_buffer, + const Array& type_definitions, + const Array& arguments, + const TypeArguments& type_arguments) const { + auto thread = Thread::Current(); + auto zone = thread->zone(); + const auto& library = Library::Handle(zone, klass.library()); + return Instance::EvaluateCompiledExpression(thread, *this, library, klass, + kernel_buffer, type_definitions, + arguments, type_arguments); +} + +ObjectPtr Instance::EvaluateCompiledExpression( + Thread* thread, + const Object& receiver, + const Library& library, + const Class& klass, + const ExternalTypedData& kernel_buffer, + const Array& type_definitions, + const Array& arguments, + const TypeArguments& type_arguments) { + auto zone = Thread::Current()->zone(); +#if defined(DART_PRECOMPILED_RUNTIME) + const auto& error_str = String::Handle( + zone, + String::New("Expression evaluation not available in precompiled mode.")); + return ApiError::New(error_str); +#else + if (IsInternalOnlyClassId(klass.id()) || (klass.id() == kTypeArgumentsCid)) { + const auto& exception = Instance::Handle( + zone, String::New("Expressions can be evaluated only with regular Dart " + "instances/classes.")); + return UnhandledException::New(exception, StackTrace::null_instance()); } - return EvaluateCompiledExpressionHelper( - kernel_buffer, type_definitions, - String::Handle(Library::Handle(library()).url()), - IsTopLevel() ? String::Handle() : String::Handle(UserVisibleName()), - arguments, type_arguments); + const auto& url = String::Handle(zone, library.url()); + const auto& klass_name = klass.IsTopLevel() + ? String::null_string() + : String::Handle(zone, klass.UserVisibleName()); + + const auto& result = Object::Handle( + zone, + LoadExpressionEvaluationFunction(zone, kernel_buffer, url, klass_name)); + if (result.IsError()) return result.ptr(); + + const auto& eval_function = Function::Cast(result); + + auto& all_arguments = Array::Handle(zone, arguments.ptr()); + if (!eval_function.is_static()) { + // `this` may be optimized out (e.g. not accessible from breakpoint due to + // not being captured by closure). We allow this as long as the evaluation + // function doesn't actually need `this`. + if (receiver.IsNull() || receiver.ptr() == Object::optimized_out().ptr()) { + if (EvaluationFunctionNeedsReceiver(thread, zone, eval_function)) { + return Object::optimized_out().ptr(); + } + } + + all_arguments = Array::New(1 + arguments.Length()); + auto& param = PassiveObject::Handle(); + all_arguments.SetAt(0, receiver); + for (intptr_t i = 0; i < arguments.Length(); i++) { + param = arguments.At(i); + all_arguments.SetAt(i + 1, param); + } + } + + return EvaluateCompiledExpressionHelper(zone, eval_function, type_definitions, + all_arguments, type_arguments); +#endif // !defined(DART_PRECOMPILED_RUNTIME) } void Class::EnsureDeclarationLoaded() const { @@ -14573,16 +14705,6 @@ ObjectPtr Library::Invoke(const String& function_name, return DartEntry::InvokeFunction(function, args, args_descriptor_array); } -ObjectPtr Library::EvaluateCompiledExpression( - const ExternalTypedData& kernel_buffer, - const Array& type_definitions, - const Array& arguments, - const TypeArguments& type_arguments) const { - return EvaluateCompiledExpressionHelper( - kernel_buffer, type_definitions, String::Handle(url()), String::Handle(), - arguments, type_arguments); -} - void Library::InitNativeWrappersLibrary(IsolateGroup* isolate_group, bool is_kernel) { const int kNumNativeWrappersClasses = 4; @@ -14638,65 +14760,6 @@ class LibraryLookupTraits { }; typedef UnorderedHashMap LibraryLookupMap; -static ObjectPtr EvaluateCompiledExpressionHelper( - const ExternalTypedData& kernel_buffer, - const Array& type_definitions, - const String& library_url, - const String& klass, - const Array& arguments, - const TypeArguments& type_arguments) { - Thread* thread = Thread::Current(); - Zone* zone = thread->zone(); -#if defined(DART_PRECOMPILED_RUNTIME) - const String& error_str = String::Handle( - zone, - String::New("Expression evaluation not available in precompiled mode.")); - return ApiError::New(error_str); -#else - std::unique_ptr kernel_pgm = - kernel::Program::ReadFromTypedData(kernel_buffer); - - if (kernel_pgm == nullptr) { - return ApiError::New(String::Handle( - zone, String::New("Kernel isolate returned ill-formed kernel."))); - } - - auto& result = Object::Handle(zone); - { - kernel::KernelLoader loader(kernel_pgm.get(), - /*uri_to_source_table=*/nullptr); - result = loader.LoadExpressionEvaluationFunction(library_url, klass); - kernel_pgm.reset(); - } - - if (result.IsError()) return result.ptr(); - - const auto& callee = Function::CheckedHandle(zone, result.ptr()); - - // type_arguments is null if all type arguments are dynamic. - if (type_definitions.Length() == 0 || type_arguments.IsNull()) { - result = DartEntry::InvokeFunction(callee, arguments); - } else { - intptr_t num_type_args = type_arguments.Length(); - Array& real_arguments = - Array::Handle(zone, Array::New(arguments.Length() + 1)); - real_arguments.SetAt(0, type_arguments); - Object& arg = Object::Handle(zone); - for (intptr_t i = 0; i < arguments.Length(); ++i) { - arg = arguments.At(i); - real_arguments.SetAt(i + 1, arg); - } - - const Array& args_desc = - Array::Handle(zone, ArgumentsDescriptor::NewBoxed( - num_type_args, arguments.Length(), Heap::kNew)); - result = DartEntry::InvokeFunction(callee, real_arguments, args_desc); - } - - return result.ptr(); -#endif -} - // Returns library with given url in current isolate, or nullptr. LibraryPtr Library::LookupLibrary(Thread* thread, const String& url) { Zone* zone = thread->zone(); @@ -20344,28 +20407,6 @@ ObjectPtr Instance::Invoke(const String& function_name, inst_type_args); } -ObjectPtr Instance::EvaluateCompiledExpression( - const Class& method_cls, - const ExternalTypedData& kernel_buffer, - const Array& type_definitions, - const Array& arguments, - const TypeArguments& type_arguments) const { - const Array& arguments_with_receiver = - Array::Handle(Array::New(1 + arguments.Length())); - PassiveObject& param = PassiveObject::Handle(); - arguments_with_receiver.SetAt(0, *this); - for (intptr_t i = 0; i < arguments.Length(); i++) { - param = arguments.At(i); - arguments_with_receiver.SetAt(i + 1, param); - } - - return EvaluateCompiledExpressionHelper( - kernel_buffer, type_definitions, - String::Handle(Library::Handle(method_cls.library()).url()), - String::Handle(method_cls.UserVisibleName()), arguments_with_receiver, - type_arguments); -} - ObjectPtr Instance::HashCode() const { // TODO(koda): Optimize for all builtin classes and all classes // that do not override hashCode. diff --git a/runtime/vm/object.h b/runtime/vm/object.h index e51cfd6e250..f8b98c2cbd8 100644 --- a/runtime/vm/object.h +++ b/runtime/vm/object.h @@ -8201,17 +8201,31 @@ class Instance : public Object { bool respect_reflectable = true, bool check_is_entrypoint = false) const; + ObjectPtr EvaluateCompiledExpression( + const Class& klass, + const ExternalTypedData& kernel_buffer, + const Array& type_definitions, + const Array& arguments, + const TypeArguments& type_arguments) const; + // Evaluate the given expression as if it appeared in an instance method of - // this instance and return the resulting value, or an error object if + // [receiver] and return the resulting value, or an error object if // evaluating the expression fails. The method has the formal (type) // parameters given in (type_)param_names, and is invoked with the (type) // argument values given in (type_)param_values. - ObjectPtr EvaluateCompiledExpression( - const Class& method_cls, + // + // We allow [receiver] to be null/ if + // * the evaluation function doesn't access `this` + // * the evaluation function is static + static ObjectPtr EvaluateCompiledExpression( + Thread* thread, + const Object& receiver, + const Library& library, + const Class& klass, const ExternalTypedData& kernel_buffer, const Array& type_definitions, const Array& param_values, - const TypeArguments& type_param_values) const; + const TypeArguments& type_param_values); // Equivalent to invoking hashCode on this instance. virtual ObjectPtr HashCode() const; diff --git a/runtime/vm/parser.h b/runtime/vm/parser.h index a801ae60b20..5237e32a9d7 100644 --- a/runtime/vm/parser.h +++ b/runtime/vm/parser.h @@ -141,6 +141,13 @@ class ParsedFunction : public ZoneAllocated { } bool has_receiver_var() const { return receiver_var_ != nullptr; } + void set_receiver_used() { receiver_used_ = true; } + bool is_receiver_used() const { + ASSERT(kernel_scopes_ != nullptr); + ASSERT(!receiver_used_ || receiver_var() != nullptr); + return receiver_used_; + } + LocalVariable* expression_temp_var() const { ASSERT(has_expression_temp_var()); return expression_temp_var_; @@ -302,6 +309,7 @@ class ParsedFunction : public ZoneAllocated { DynamicClosureCallVars* dynamic_closure_call_vars_; mutable FieldSet guarded_fields_; ZoneGrowableArray* default_parameter_values_; + bool receiver_used_ = false; LocalVariable* raw_type_arguments_var_; ZoneGrowableArray* raw_parameters_ = nullptr; diff --git a/runtime/vm/scopes.cc b/runtime/vm/scopes.cc index 846a49bdd76..35701f1d934 100644 --- a/runtime/vm/scopes.cc +++ b/runtime/vm/scopes.cc @@ -341,7 +341,7 @@ LocalVariable* LocalScope::LookupVariable(const String& name, LocalVariable* var = current_scope->LocalLookupVariable(name, kernel_offset); // If testing only, return the variable even if invisible. - if ((var != nullptr) && (!var->is_invisible_ || test_only)) { + if ((var != nullptr) && (!var->is_invisible() || test_only)) { if (!test_only && (var->owner()->function_level() != function_level())) { CaptureVariable(var); } diff --git a/runtime/vm/scopes.h b/runtime/vm/scopes.h index b1979da428a..5e11a570737 100644 --- a/runtime/vm/scopes.h +++ b/runtime/vm/scopes.h @@ -91,13 +91,7 @@ class LocalVariable : public ZoneAllocated { parameter_type_(parameter_type), parameter_value_(parameter_value), const_value_(nullptr), - is_final_(false), - is_captured_(false), - is_invisible_(false), - is_captured_parameter_(false), - is_forced_stack_(false), covariance_mode_(kNotCovariant), - is_late_(false), late_init_offset_(0), type_check_mode_(kDoTypeCheck), index_(), @@ -132,11 +126,11 @@ class LocalVariable : public ZoneAllocated { CompileType* parameter_type() const { return parameter_type_; } const Object* parameter_value() const { return parameter_value_; } - bool is_final() const { return is_final_; } - void set_is_final() { is_final_ = true; } + bool is_final() const { return IsFinalBit::decode(bitfield_); } + void set_is_final() { bitfield_ = IsFinalBit::update(true, bitfield_); } - bool is_captured() const { return is_captured_; } - void set_is_captured() { is_captured_ = true; } + bool is_captured() const { return IsCapturedBit::decode(bitfield_); } + void set_is_captured() { bitfield_ = IsCapturedBit::update(true, bitfield_); } bool ComputeIfIsAwaiterLink(const Library& library); void set_is_awaiter_link(bool value) { @@ -147,11 +141,13 @@ class LocalVariable : public ZoneAllocated { // CaptureLocalVariables - which iterates scope chain between two scopes // and indiscriminately marks all variables as captured. // TODO(27590) remove the hardcoded list of names from CaptureLocalVariables - bool is_forced_stack() const { return is_forced_stack_; } - void set_is_forced_stack() { is_forced_stack_ = true; } + bool is_forced_stack() const { return IsForcedStackBit::decode(bitfield_); } + void set_is_forced_stack() { + bitfield_ = IsForcedStackBit::update(true, bitfield_); + } - bool is_late() const { return is_late_; } - void set_is_late() { is_late_ = true; } + bool is_late() const { return IsLateBit::decode(bitfield_); } + void set_is_late() { bitfield_ = IsLateBit::update(true, bitfield_); } intptr_t late_init_offset() const { return late_init_offset_; } void set_late_init_offset(intptr_t late_init_offset) { @@ -205,11 +201,17 @@ class LocalVariable : public ZoneAllocated { // Invisible variables are not included into LocalVarDescriptors // and not displayed in the debugger. - void set_invisible(bool value) { is_invisible_ = value; } - bool is_invisible() const { return is_invisible_; } + bool is_invisible() const { return IsInvisibleBit::decode(bitfield_); } + void set_invisible(bool value) { + bitfield_ = IsInvisibleBit::update(value, bitfield_); + } - bool is_captured_parameter() const { return is_captured_parameter_; } - void set_is_captured_parameter(bool value) { is_captured_parameter_ = value; } + bool is_captured_parameter() const { + return IsCapturedParameterBit::decode(bitfield_); + } + void set_is_captured_parameter(bool value) { + bitfield_ = IsCapturedParameterBit::update(value, bitfield_); + } bool IsConst() const { return const_value_ != nullptr; } @@ -226,6 +228,18 @@ class LocalVariable : public ZoneAllocated { bool Equals(const LocalVariable& other) const; private: + // If true, this variable is readonly. + using IsFinalBit = BitField; + // If true, this variable lives in the context, otherwise + // in the stack frame. + using IsCapturedBit = BitField; + using IsInvisibleBit = BitField; + using IsCapturedParameterBit = + BitField; + using IsForcedStackBit = + BitField; + using IsLateBit = BitField; + enum CovarianceMode { kNotCovariant, kImplicit, @@ -250,14 +264,8 @@ class LocalVariable : public ZoneAllocated { const Instance* const_value_; // nullptr or compile-time const value. - bool is_final_; // If true, this variable is readonly. - bool is_captured_; // If true, this variable lives in the context, otherwise - // in the stack frame. - bool is_invisible_; - bool is_captured_parameter_; - bool is_forced_stack_; + uint32_t bitfield_ = 0; CovarianceMode covariance_mode_; - bool is_late_; intptr_t late_init_offset_; TypeCheckMode type_check_mode_; VariableIndex index_; diff --git a/runtime/vm/service.cc b/runtime/vm/service.cc index 3ea34acc11a..ae088212646 100644 --- a/runtime/vm/service.cc +++ b/runtime/vm/service.cc @@ -3370,43 +3370,38 @@ static void EvaluateCompiledExpression(Thread* thread, JSONStream* js) { } return; } + const auto& type_params_names_fixed = + Array::Handle(zone, Array::MakeFixedLength(type_params_names)); + const auto& param_values_fixed = + Array::Handle(zone, Array::MakeFixedLength(param_values)); + TypeArguments& type_arguments = TypeArguments::Handle(zone); if (obj.IsLibrary()) { - const Library& lib = Library::Cast(obj); - const Object& result = Object::Handle( + const auto& lib = Library::Cast(obj); + const auto& result = Object::Handle( zone, - lib.EvaluateCompiledExpression( - kernel_data, - Array::Handle(zone, Array::MakeFixedLength(type_params_names)), - Array::Handle(zone, Array::MakeFixedLength(param_values)), - type_arguments)); + lib.EvaluateCompiledExpression(kernel_data, type_params_names_fixed, + param_values_fixed, type_arguments)); result.PrintJSON(js, true); return; } if (obj.IsClass()) { - const Class& cls = Class::Cast(obj); - const Object& result = Object::Handle( + const auto& cls = Class::Cast(obj); + const auto& result = Object::Handle( zone, - cls.EvaluateCompiledExpression( - kernel_data, - Array::Handle(zone, Array::MakeFixedLength(type_params_names)), - Array::Handle(zone, Array::MakeFixedLength(param_values)), - type_arguments)); + cls.EvaluateCompiledExpression(kernel_data, type_params_names_fixed, + param_values_fixed, type_arguments)); result.PrintJSON(js, true); return; } if ((obj.IsInstance() || obj.IsNull()) && !ContainsNonInstance(obj)) { - // We don't use Instance::Cast here because it doesn't allow null. - Instance& instance = Instance::Handle(zone); - instance ^= obj.ptr(); - const Class& receiver_cls = Class::Handle(zone, instance.clazz()); - const Object& result = Object::Handle( - zone, - instance.EvaluateCompiledExpression( - receiver_cls, kernel_data, - Array::Handle(zone, Array::MakeFixedLength(type_params_names)), - Array::Handle(zone, Array::MakeFixedLength(param_values)), - type_arguments)); + const auto& instance = + Instance::Handle(zone, Instance::RawCast(obj.ptr())); + const auto& receiver_cls = Class::Handle(zone, instance.clazz()); + const auto& result = Object::Handle( + zone, instance.EvaluateCompiledExpression( + receiver_cls, kernel_data, type_params_names_fixed, + param_values_fixed, type_arguments)); result.PrintJSON(js, true); return; }