[vm/ffi] Fix LR clobber in JumpToFrame stub on ARM
Both ARM and ARM64 variants did not account for the
fact that TransitionNativeToGenerated will clobber
LR on slow path.
TEST=tests/ffi/regress_jump_to_frame_test.dart
Cq-Include-Trybots: luci.dart.try:vm-ffi-android-release-arm-try,vm-ffi-android-release-arm64-try,vm-kernel-linux-release-simarm64-try,vm-kernel-linux-release-simarm-try,vm-kernel-precomp-linux-release-simarm-try,vm-kernel-precomp-linux-release-simarm64-try
Change-Id: I40d48b720b2a7f31ff20903a887295ab2034662c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/173962
Commit-Queue: Vyacheslav Egorov <vegorov@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
diff --git a/runtime/vm/compiler/asm_intrinsifier_arm.cc b/runtime/vm/compiler/asm_intrinsifier_arm.cc
index 26a7f91..fdfb79d 100644
--- a/runtime/vm/compiler/asm_intrinsifier_arm.cc
+++ b/runtime/vm/compiler/asm_intrinsifier_arm.cc
@@ -28,14 +28,10 @@
return -1;
}
-static bool IsABIPreservedRegister(Register reg) {
- return ((1 << reg) & kAbiPreservedCpuRegs) != 0;
-}
-
void AsmIntrinsifier::IntrinsicCallPrologue(Assembler* assembler) {
- ASSERT(IsABIPreservedRegister(CODE_REG));
- ASSERT(IsABIPreservedRegister(ARGS_DESC_REG));
- ASSERT(IsABIPreservedRegister(CALLEE_SAVED_TEMP));
+ COMPILE_ASSERT(IsAbiPreservedRegister(CODE_REG));
+ COMPILE_ASSERT(IsAbiPreservedRegister(ARGS_DESC_REG));
+ COMPILE_ASSERT(IsAbiPreservedRegister(CALLEE_SAVED_TEMP));
// Save LR by moving it to a callee saved temporary register.
assembler->Comment("IntrinsicCallPrologue");
diff --git a/runtime/vm/compiler/asm_intrinsifier_arm64.cc b/runtime/vm/compiler/asm_intrinsifier_arm64.cc
index 14857e9..e218d71 100644
--- a/runtime/vm/compiler/asm_intrinsifier_arm64.cc
+++ b/runtime/vm/compiler/asm_intrinsifier_arm64.cc
@@ -28,19 +28,15 @@
return -1;
}
-static bool IsABIPreservedRegister(Register reg) {
- return ((1 << reg) & kAbiPreservedCpuRegs) != 0;
-}
-
void AsmIntrinsifier::IntrinsicCallPrologue(Assembler* assembler) {
- ASSERT(IsABIPreservedRegister(CODE_REG));
- ASSERT(!IsABIPreservedRegister(ARGS_DESC_REG));
- ASSERT(IsABIPreservedRegister(CALLEE_SAVED_TEMP));
- ASSERT(IsABIPreservedRegister(CALLEE_SAVED_TEMP2));
- ASSERT(CALLEE_SAVED_TEMP != CODE_REG);
- ASSERT(CALLEE_SAVED_TEMP != ARGS_DESC_REG);
- ASSERT(CALLEE_SAVED_TEMP2 != CODE_REG);
- ASSERT(CALLEE_SAVED_TEMP2 != ARGS_DESC_REG);
+ COMPILE_ASSERT(IsAbiPreservedRegister(CODE_REG));
+ COMPILE_ASSERT(!IsAbiPreservedRegister(ARGS_DESC_REG));
+ COMPILE_ASSERT(IsAbiPreservedRegister(CALLEE_SAVED_TEMP));
+ COMPILE_ASSERT(IsAbiPreservedRegister(CALLEE_SAVED_TEMP2));
+ COMPILE_ASSERT(CALLEE_SAVED_TEMP != CODE_REG);
+ COMPILE_ASSERT(CALLEE_SAVED_TEMP != ARGS_DESC_REG);
+ COMPILE_ASSERT(CALLEE_SAVED_TEMP2 != CODE_REG);
+ COMPILE_ASSERT(CALLEE_SAVED_TEMP2 != ARGS_DESC_REG);
assembler->Comment("IntrinsicCallPrologue");
assembler->mov(CALLEE_SAVED_TEMP, LR);
diff --git a/runtime/vm/compiler/asm_intrinsifier_x64.cc b/runtime/vm/compiler/asm_intrinsifier_x64.cc
index ac7112c..f13d41f 100644
--- a/runtime/vm/compiler/asm_intrinsifier_x64.cc
+++ b/runtime/vm/compiler/asm_intrinsifier_x64.cc
@@ -28,16 +28,12 @@
return 0;
}
-static bool IsABIPreservedRegister(Register reg) {
- return ((1 << reg) & CallingConventions::kCalleeSaveCpuRegisters) != 0;
-}
-
void AsmIntrinsifier::IntrinsicCallPrologue(Assembler* assembler) {
- ASSERT(IsABIPreservedRegister(CODE_REG));
- ASSERT(!IsABIPreservedRegister(ARGS_DESC_REG));
- ASSERT(IsABIPreservedRegister(CALLEE_SAVED_TEMP));
- ASSERT(CALLEE_SAVED_TEMP != CODE_REG);
- ASSERT(CALLEE_SAVED_TEMP != ARGS_DESC_REG);
+ COMPILE_ASSERT(IsAbiPreservedRegister(CODE_REG));
+ COMPILE_ASSERT(!IsAbiPreservedRegister(ARGS_DESC_REG));
+ COMPILE_ASSERT(IsAbiPreservedRegister(CALLEE_SAVED_TEMP));
+ COMPILE_ASSERT(CALLEE_SAVED_TEMP != CODE_REG);
+ COMPILE_ASSERT(CALLEE_SAVED_TEMP != ARGS_DESC_REG);
assembler->Comment("IntrinsicCallPrologue");
assembler->movq(CALLEE_SAVED_TEMP, ARGS_DESC_REG);
diff --git a/runtime/vm/compiler/graph_intrinsifier_arm.cc b/runtime/vm/compiler/graph_intrinsifier_arm.cc
index cc8dc4c..1cebb33 100644
--- a/runtime/vm/compiler/graph_intrinsifier_arm.cc
+++ b/runtime/vm/compiler/graph_intrinsifier_arm.cc
@@ -17,14 +17,10 @@
return -1;
}
-static bool IsABIPreservedRegister(Register reg) {
- return ((1 << reg) & kAbiPreservedCpuRegs) != 0;
-}
-
void GraphIntrinsifier::IntrinsicCallPrologue(Assembler* assembler) {
- ASSERT(IsABIPreservedRegister(CODE_REG));
- ASSERT(IsABIPreservedRegister(ARGS_DESC_REG));
- ASSERT(IsABIPreservedRegister(CALLEE_SAVED_TEMP));
+ COMPILE_ASSERT(IsAbiPreservedRegister(CODE_REG));
+ COMPILE_ASSERT(IsAbiPreservedRegister(ARGS_DESC_REG));
+ COMPILE_ASSERT(IsAbiPreservedRegister(CALLEE_SAVED_TEMP));
// Save LR by moving it to a callee saved temporary register.
assembler->Comment("IntrinsicCallPrologue");
diff --git a/runtime/vm/compiler/graph_intrinsifier_arm64.cc b/runtime/vm/compiler/graph_intrinsifier_arm64.cc
index 9bd2ae5..99d33b4 100644
--- a/runtime/vm/compiler/graph_intrinsifier_arm64.cc
+++ b/runtime/vm/compiler/graph_intrinsifier_arm64.cc
@@ -17,19 +17,15 @@
return -1;
}
-static bool IsABIPreservedRegister(Register reg) {
- return ((1 << reg) & kAbiPreservedCpuRegs) != 0;
-}
-
void GraphIntrinsifier::IntrinsicCallPrologue(Assembler* assembler) {
- ASSERT(IsABIPreservedRegister(CODE_REG));
- ASSERT(!IsABIPreservedRegister(ARGS_DESC_REG));
- ASSERT(IsABIPreservedRegister(CALLEE_SAVED_TEMP));
- ASSERT(IsABIPreservedRegister(CALLEE_SAVED_TEMP2));
- ASSERT(CALLEE_SAVED_TEMP != CODE_REG);
- ASSERT(CALLEE_SAVED_TEMP != ARGS_DESC_REG);
- ASSERT(CALLEE_SAVED_TEMP2 != CODE_REG);
- ASSERT(CALLEE_SAVED_TEMP2 != ARGS_DESC_REG);
+ COMPILE_ASSERT(IsAbiPreservedRegister(CODE_REG));
+ COMPILE_ASSERT(!IsAbiPreservedRegister(ARGS_DESC_REG));
+ COMPILE_ASSERT(IsAbiPreservedRegister(CALLEE_SAVED_TEMP));
+ COMPILE_ASSERT(IsAbiPreservedRegister(CALLEE_SAVED_TEMP2));
+ COMPILE_ASSERT(CALLEE_SAVED_TEMP != CODE_REG);
+ COMPILE_ASSERT(CALLEE_SAVED_TEMP != ARGS_DESC_REG);
+ COMPILE_ASSERT(CALLEE_SAVED_TEMP2 != CODE_REG);
+ COMPILE_ASSERT(CALLEE_SAVED_TEMP2 != ARGS_DESC_REG);
assembler->Comment("IntrinsicCallPrologue");
assembler->mov(CALLEE_SAVED_TEMP, LR);
diff --git a/runtime/vm/compiler/graph_intrinsifier_x64.cc b/runtime/vm/compiler/graph_intrinsifier_x64.cc
index 58c02b2..e1479a8 100644
--- a/runtime/vm/compiler/graph_intrinsifier_x64.cc
+++ b/runtime/vm/compiler/graph_intrinsifier_x64.cc
@@ -17,16 +17,12 @@
return 0;
}
-static bool IsABIPreservedRegister(Register reg) {
- return ((1 << reg) & CallingConventions::kCalleeSaveCpuRegisters) != 0;
-}
-
void GraphIntrinsifier::IntrinsicCallPrologue(Assembler* assembler) {
- ASSERT(IsABIPreservedRegister(CODE_REG));
- ASSERT(!IsABIPreservedRegister(ARGS_DESC_REG));
- ASSERT(IsABIPreservedRegister(CALLEE_SAVED_TEMP));
- ASSERT(CALLEE_SAVED_TEMP != CODE_REG);
- ASSERT(CALLEE_SAVED_TEMP != ARGS_DESC_REG);
+ COMPILE_ASSERT(IsAbiPreservedRegister(CODE_REG));
+ COMPILE_ASSERT(!IsAbiPreservedRegister(ARGS_DESC_REG));
+ COMPILE_ASSERT(IsAbiPreservedRegister(CALLEE_SAVED_TEMP));
+ COMPILE_ASSERT(CALLEE_SAVED_TEMP != CODE_REG);
+ COMPILE_ASSERT(CALLEE_SAVED_TEMP != ARGS_DESC_REG);
assembler->Comment("IntrinsicCallPrologue");
assembler->movq(CALLEE_SAVED_TEMP, ARGS_DESC_REG);
diff --git a/runtime/vm/compiler/stub_code_compiler_arm.cc b/runtime/vm/compiler/stub_code_compiler_arm.cc
index 4bc6aee..5bf87cb 100644
--- a/runtime/vm/compiler/stub_code_compiler_arm.cc
+++ b/runtime/vm/compiler/stub_code_compiler_arm.cc
@@ -350,7 +350,7 @@
// NOTFP, R4: clobbered, although normally callee-saved
void StubCodeCompiler::GenerateCallNativeThroughSafepointStub(
Assembler* assembler) {
- COMPILE_ASSERT((kAbiPreservedCpuRegs & (1 << R4)) != 0);
+ COMPILE_ASSERT(IsAbiPreservedRegister(R4));
// TransitionGeneratedToNative might clobber LR if it takes the slow path.
__ mov(R4, Operand(LR));
@@ -2867,10 +2867,13 @@
//
// Notice: We need to keep this in sync with `Simulator::JumpToFrame()`.
void StubCodeCompiler::GenerateJumpToFrameStub(Assembler* assembler) {
- ASSERT(kExceptionObjectReg == R0);
- ASSERT(kStackTraceObjectReg == R1);
+ COMPILE_ASSERT(kExceptionObjectReg == R0);
+ COMPILE_ASSERT(kStackTraceObjectReg == R1);
+ COMPILE_ASSERT(IsAbiPreservedRegister(R4));
+ COMPILE_ASSERT(IsAbiPreservedRegister(THR));
__ mov(IP, Operand(R1)); // Copy Stack pointer into IP.
- __ mov(LR, Operand(R0)); // Program counter.
+ // TransitionGeneratedToNative might clobber LR if it takes the slow path.
+ __ mov(R4, Operand(R0)); // Program counter.
__ mov(THR, Operand(R3)); // Thread.
__ mov(FP, Operand(R2)); // Frame_pointer.
__ mov(SP, Operand(IP)); // Set Stack pointer.
@@ -2903,7 +2906,7 @@
} else {
__ LoadPoolPointer();
}
- __ bx(LR); // Jump to continuation point.
+ __ bx(R4); // Jump to continuation point.
}
// Run an exception handler. Execution comes from JumpToFrame
diff --git a/runtime/vm/compiler/stub_code_compiler_arm64.cc b/runtime/vm/compiler/stub_code_compiler_arm64.cc
index 1ef8b6a..f98d773 100644
--- a/runtime/vm/compiler/stub_code_compiler_arm64.cc
+++ b/runtime/vm/compiler/stub_code_compiler_arm64.cc
@@ -300,7 +300,7 @@
// Stack: preserved, CSP == SP
void StubCodeCompiler::GenerateCallNativeThroughSafepointStub(
Assembler* assembler) {
- COMPILE_ASSERT((1 << R19) & kAbiPreservedCpuRegs);
+ COMPILE_ASSERT(IsAbiPreservedRegister(R19));
__ mov(R19, LR);
__ LoadImmediate(R10, target::Thread::exit_through_ffi());
@@ -3007,7 +3007,8 @@
void StubCodeCompiler::GenerateJumpToFrameStub(Assembler* assembler) {
ASSERT(kExceptionObjectReg == R0);
ASSERT(kStackTraceObjectReg == R1);
- __ mov(LR, R0); // Program counter.
+ // TransitionGeneratedToNative might clobber LR if it takes the slow path.
+ __ mov(CALLEE_SAVED_TEMP, R0); // Program counter.
__ mov(SP, R1); // Stack pointer.
__ mov(FP, R2); // Frame_pointer.
__ mov(THR, R3);
@@ -3042,7 +3043,7 @@
} else {
__ LoadPoolPointer();
}
- __ ret(); // Jump to continuation point.
+ __ ret(CALLEE_SAVED_TEMP); // Jump to continuation point.
}
// Run an exception handler. Execution comes from JumpToFrame
diff --git a/runtime/vm/constants.h b/runtime/vm/constants.h
index f89abc6..01d9d08 100644
--- a/runtime/vm/constants.h
+++ b/runtime/vm/constants.h
@@ -53,6 +53,12 @@
return ((1 << reg) & CallingConventions::kCalleeSaveCpuRegisters) != 0;
}
+#if !defined(TARGET_ARCH_IA32)
+constexpr bool IsAbiPreservedRegister(Register reg) {
+ return (kAbiPreservedCpuRegs & (1 << reg)) != 0;
+}
+#endif
+
} // namespace dart
#endif // RUNTIME_VM_CONSTANTS_H_
diff --git a/runtime/vm/constants_x64.h b/runtime/vm/constants_x64.h
index 824a871..480f565 100644
--- a/runtime/vm/constants_x64.h
+++ b/runtime/vm/constants_x64.h
@@ -457,6 +457,9 @@
(kArgumentRegisters | R(kPointerToReturnStructRegisterCall))) == 0);
};
+constexpr intptr_t kAbiPreservedCpuRegs =
+ CallingConventions::kCalleeSaveCpuRegisters;
+
#undef R
class Instr {
diff --git a/runtime/vm/runtime_entry_arm.cc b/runtime/vm/runtime_entry_arm.cc
index b2cd9da..0870d45 100644
--- a/runtime/vm/runtime_entry_arm.cc
+++ b/runtime/vm/runtime_entry_arm.cc
@@ -58,8 +58,8 @@
__ LoadImmediate(TMP, VMTag::kDartTagId);
__ str(TMP,
compiler::Address(THR, compiler::target::Thread::vm_tag_offset()));
- ASSERT((kAbiPreservedCpuRegs & (1 << THR)) != 0);
- ASSERT((kAbiPreservedCpuRegs & (1 << PP)) != 0);
+ COMPILE_ASSERT(IsAbiPreservedRegister(THR));
+ COMPILE_ASSERT(IsAbiPreservedRegister(PP));
} else {
// Argument count is not checked here, but in the runtime entry for a more
// informative error message.
diff --git a/runtime/vm/runtime_entry_arm64.cc b/runtime/vm/runtime_entry_arm64.cc
index 1580e1a..93a80fd 100644
--- a/runtime/vm/runtime_entry_arm64.cc
+++ b/runtime/vm/runtime_entry_arm64.cc
@@ -73,8 +73,8 @@
__ str(TMP, compiler::Address(THR, Thread::vm_tag_offset()));
__ mov(SP, kCallLeafRuntimeCalleeSaveScratch2);
__ mov(CSP, kCallLeafRuntimeCalleeSaveScratch1);
- ASSERT((kAbiPreservedCpuRegs & (1 << THR)) != 0);
- ASSERT((kAbiPreservedCpuRegs & (1 << PP)) != 0);
+ COMPILE_ASSERT(IsAbiPreservedRegister(THR));
+ COMPILE_ASSERT(IsAbiPreservedRegister(PP));
} else {
// Argument count is not checked here, but in the runtime entry for a more
// informative error message.
diff --git a/tests/ffi/regress_jump_to_frame_test.dart b/tests/ffi/regress_jump_to_frame_test.dart
new file mode 100644
index 0000000..a3d9054
--- /dev/null
+++ b/tests/ffi/regress_jump_to_frame_test.dart
@@ -0,0 +1,64 @@
+// Copyright (c) 2020, 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.
+
+// Check that JumpToFrame does not use LR clobbered by slow path of
+// TransitionNativeToGenerated.
+// VMOptions=--use-slow-path --enable-testing-pragmas
+
+import 'dart:ffi';
+
+import 'package:ffi/ffi.dart';
+import 'package:expect/expect.dart';
+
+typedef CallbackDartType = Object Function();
+typedef CallbackNativeType = Handle Function();
+
+Object alwaysThrows() {
+ throw 'exception';
+}
+
+void alwaysCatches(CallbackDartType f) {
+ try {
+ propagateError(f());
+ } catch (e) {
+ Expect.equals('exception', e);
+ return;
+ }
+ Expect.isTrue(false);
+}
+
+void main() {
+ final ptr = Pointer.fromFunction<CallbackNativeType>(alwaysThrows);
+ final f = ptr.asFunction<CallbackDartType>();
+ alwaysCatches(f);
+}
+
+typedef Dart_PropagateError_NativeType = Void Function(Handle);
+typedef Dart_PropagateError_DartType = void Function(Object);
+
+final Dart_PropagateError_DartType propagateError = () {
+ final Pointer<_DartApi> dlapi = NativeApi.initializeApiDLData.cast();
+ for (int i = 0; dlapi.ref.functions[i].name != nullptr; i++) {
+ final name = Utf8.fromUtf8(dlapi.ref.functions[i].name.cast<Utf8>());
+ if (name == 'Dart_PropagateError') {
+ return dlapi.ref.functions[i].function
+ .cast<NativeFunction<Dart_PropagateError_NativeType>>()
+ .asFunction<Dart_PropagateError_DartType>();
+ }
+ }
+ throw 'Not found';
+}();
+
+class _DartEntry extends Struct {
+ external Pointer<Int8> name;
+ external Pointer<Void> function;
+}
+
+class _DartApi extends Struct {
+ @Int32()
+ external int major;
+ @Int32()
+ external int minor;
+ external Pointer<_DartEntry> functions;
+}
diff --git a/tests/ffi_2/regress_jump_to_frame_test.dart b/tests/ffi_2/regress_jump_to_frame_test.dart
new file mode 100644
index 0000000..9838443
--- /dev/null
+++ b/tests/ffi_2/regress_jump_to_frame_test.dart
@@ -0,0 +1,64 @@
+// Copyright (c) 2020, 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.
+
+// Check that JumpToFrame does not use LR clobbered by slow path of
+// TransitionNativeToGenerated.
+// VMOptions=--use-slow-path --enable-testing-pragmas
+
+import 'dart:ffi';
+
+import 'package:ffi/ffi.dart';
+import 'package:expect/expect.dart';
+
+typedef CallbackDartType = Object Function();
+typedef CallbackNativeType = Handle Function();
+
+Object alwaysThrows() {
+ throw 'exception';
+}
+
+void alwaysCatches(CallbackDartType f) {
+ try {
+ propagateError(f());
+ } catch (e) {
+ Expect.equals('exception', e);
+ return;
+ }
+ Expect.isTrue(false);
+}
+
+void main() {
+ final ptr = Pointer.fromFunction<CallbackNativeType>(alwaysThrows);
+ final f = ptr.asFunction<CallbackDartType>();
+ alwaysCatches(f);
+}
+
+typedef Dart_PropagateError_NativeType = Void Function(Handle);
+typedef Dart_PropagateError_DartType = void Function(Object);
+
+final Dart_PropagateError_DartType propagateError = () {
+ final Pointer<_DartApi> dlapi = NativeApi.initializeApiDLData.cast();
+ for (int i = 0; dlapi.ref.functions[i].name != nullptr; i++) {
+ final name = Utf8.fromUtf8(dlapi.ref.functions[i].name.cast<Utf8>());
+ if (name == 'Dart_PropagateError') {
+ return dlapi.ref.functions[i].function
+ .cast<NativeFunction<Dart_PropagateError_NativeType>>()
+ .asFunction<Dart_PropagateError_DartType>();
+ }
+ }
+ throw 'Not found';
+}();
+
+class _DartEntry extends Struct {
+ Pointer<Int8> name;
+ Pointer<Void> function;
+}
+
+class _DartApi extends Struct {
+ @Int32()
+ int major;
+ @Int32()
+ int minor;
+ Pointer<_DartEntry> functions;
+}
