From ec9a660ccca94edaf04051192282c7ecaf2bcccc Mon Sep 17 00:00:00 2001 From: Vyacheslav Egorov Date: Fri, 1 Jul 2022 12:31:47 +0000 Subject: [PATCH] [vm] Avoid clobbering array length in AllocateArray stub on ARM It was calling EnsureIsNewOrRemembered on the slow path which was forgetting to preserve registers around a runtime call. Fixes https://github.com/flutter/flutter/issues/106510 TEST=vm/dart{,_2}/flutter_regress_106510 Cq-Include-Trybots: luci.dart.try:vm-kernel-linux-release-simarm-try,vm-kernel-precomp-linux-release-simarm-try Change-Id: I621e392304fcd1fd643c009fbcde3f88b6f19b7f Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/250168 Reviewed-by: Ryan Macnak Commit-Queue: Slava Egorov --- .../vm/dart/flutter_regress_106510_test.dart | 49 ++++++++++++++++++ .../dart_2/flutter_regress_106510_test.dart | 50 +++++++++++++++++++ runtime/vm/compiler/stub_code_compiler_arm.cc | 2 +- 3 files changed, 100 insertions(+), 1 deletion(-) create mode 100644 runtime/tests/vm/dart/flutter_regress_106510_test.dart create mode 100644 runtime/tests/vm/dart_2/flutter_regress_106510_test.dart diff --git a/runtime/tests/vm/dart/flutter_regress_106510_test.dart b/runtime/tests/vm/dart/flutter_regress_106510_test.dart new file mode 100644 index 00000000000..7c41c20d793 --- /dev/null +++ b/runtime/tests/vm/dart/flutter_regress_106510_test.dart @@ -0,0 +1,49 @@ +// Copyright (c) 2022, 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=--use_slow_path --stress_write_barrier_elimination + +// Test that array allocation in noSuchMethod dispatcher is preserving the +// length register correctly. + +import 'dart:async'; + +import 'package:expect/expect.dart'; + +final List list = []; + +final List invocations = []; + +abstract class B { + void someMethod(int v); +} + +class A { + noSuchMethod(Invocation i) { + Expect.equals(#someMethod, i.memberName); + invocations.add(i.positionalArguments[0] as int); + } +} + +class C implements B { + void someMethod(int v) { + invocations.add(v); + } +} + +void main() { + for (var i = 0; i < 10; i++) { + list.add(A()); + list.add(C()); + } + + for (var i = 0; i < list.length; i++) { + list[i].someMethod(i); + } + + Expect.equals(list.length, invocations.length); + for (var i = 0; i < list.length; i++) { + Expect.equals(i, invocations[i]); + } +} diff --git a/runtime/tests/vm/dart_2/flutter_regress_106510_test.dart b/runtime/tests/vm/dart_2/flutter_regress_106510_test.dart new file mode 100644 index 00000000000..5139e5783e8 --- /dev/null +++ b/runtime/tests/vm/dart_2/flutter_regress_106510_test.dart @@ -0,0 +1,50 @@ +// Copyright (c) 2022, 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. +// @dart=2.9 + +// VMOptions=--use_slow_path --stress_write_barrier_elimination + +// Test that array allocation in noSuchMethod dispatcher is preserving the +// length register correctly. + +import 'dart:async'; + +import 'package:expect/expect.dart'; + +final List list = []; + +final List invocations = []; + +abstract class B { + void someMethod(int v); +} + +class A { + noSuchMethod(Invocation i) { + Expect.equals(#someMethod, i.memberName); + invocations.add(i.positionalArguments[0] as int); + } +} + +class C implements B { + void someMethod(int v) { + invocations.add(v); + } +} + +void main() { + for (var i = 0; i < 10; i++) { + list.add(A()); + list.add(C()); + } + + for (var i = 0; i < list.length; i++) { + list[i].someMethod(i); + } + + Expect.equals(list.length, invocations.length); + for (var i = 0; i < list.length; i++) { + Expect.equals(i, invocations[i]); + } +} diff --git a/runtime/vm/compiler/stub_code_compiler_arm.cc b/runtime/vm/compiler/stub_code_compiler_arm.cc index fc6b0ea553e..6f624566f02 100644 --- a/runtime/vm/compiler/stub_code_compiler_arm.cc +++ b/runtime/vm/compiler/stub_code_compiler_arm.cc @@ -46,7 +46,7 @@ void StubCodeCompiler::EnsureIsNewOrRemembered(Assembler* assembler, { LeafRuntimeScope rt(assembler, /*frame_size=*/0, - /*preserve_registers=*/false); + /*preserve_registers=*/preserve_registers); // [R0] already contains first argument. __ mov(R1, Operand(THR)); rt.Call(kEnsureRememberedAndMarkingDeferredRuntimeEntry, 2);