Skip to content

Commit 5984cc8

Browse files
dcharkesdcharkes
committed
[vm] Verify transitions in generated code
The transitions in C++ have asserts which run in debug mode to validate that the transitions are happening in the right order. The transitions in the generated code had no equivalent. This CL introduces the equivalent for the generated code. The transitions to generated code are misnamed currently. They come from either VM or Native, not only Native. For example the FFI callbacks and call return sequence can already transition from native into the VM when entering the safepoint. These checks don't catch the invalid FFI callbacks. Bug: #60021 These are caught when looking up the FFI metadata: https://dart-review.googlesource.com/c/sdk/+/407023 This CL omits to implement the check in ia32, since ia32 is scheduled for removal. The check is only enabled in debug mode. The asserts for the C++ are also only run in debug mode. And enabling this check in release for AOT regresses the parent CL benchmark by ~30% on Mac Arm64. TEST=test/ffi Change-Id: Ieaf1e43533baae294af37b2e171b68bd314fdbe0 Cq-Include-Trybots: dart/try:vm-aot-android-release-arm64c-try,vm-aot-android-release-arm_x64-try,vm-aot-asan-linux-release-x64-try,vm-aot-linux-debug-x64-try,vm-aot-linux-debug-x64c-try,vm-aot-mac-release-arm64-try,vm-aot-mac-release-x64-try,vm-aot-msan-linux-release-x64-try,vm-aot-obfuscate-linux-release-x64-try,vm-aot-optimization-level-linux-release-x64-try,vm-aot-tsan-linux-release-x64-try,vm-aot-ubsan-linux-release-x64-try,vm-aot-win-debug-arm64-try,vm-aot-win-debug-x64-try,vm-aot-win-debug-x64c-try,vm-appjit-linux-debug-x64-try,vm-asan-linux-release-arm64-try,vm-asan-linux-release-x64-try,vm-checked-mac-release-arm64-try,vm-ffi-android-debug-arm-try,vm-ffi-android-debug-arm64c-try,vm-ffi-qemu-linux-release-arm-try,vm-ffi-qemu-linux-release-riscv64-try,vm-fuchsia-release-arm64-try,vm-fuchsia-release-x64-try,vm-linux-debug-ia32-try,vm-linux-debug-x64-try,vm-linux-debug-x64c-try,vm-mac-debug-arm64-try,vm-mac-debug-x64-try,vm-msan-linux-release-arm64-try,vm-msan-linux-release-x64-try,vm-reload-linux-debug-x64-try,vm-reload-rollback-linux-debug-x64-try,vm-tsan-linux-release-arm64-try,vm-tsan-linux-release-x64-try,vm-ubsan-linux-release-arm64-try,vm-ubsan-linux-release-x64-try,vm-win-debug-arm64-try,vm-win-debug-x64-try,vm-win-debug-x64c-try,vm-win-release-ia32-try Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/407021 Reviewed-by: Liam Appelbe <[email protected]> Reviewed-by: Slava Egorov <[email protected]>
1 parent 6157f45 commit 5984cc8

15 files changed

+145
-14
lines changed

‎runtime/vm/compiler/assembler/assembler_arm.cc‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -623,6 +623,7 @@ void Assembler::TransitionGeneratedToNative(Register destination_address,
623623
target::Thread::exit_through_ffi_offset());
624624
Register tmp2 = exit_through_ffi;
625625

626+
VerifyInGenerated(tmp1);
626627
// Mark that the thread is executing native code.
627628
StoreToOffset(destination_address, THR, target::Thread::vm_tag_offset());
628629
LoadImmediate(tmp1, target::Thread::native_execution_state());
@@ -700,6 +701,7 @@ void Assembler::TransitionNativeToGenerated(Register addr,
700701
#endif
701702
}
702703

704+
VerifyNotInGenerated(TMP);
703705
// Mark that the thread is executing Dart code.
704706
if (set_tag) {
705707
LoadImmediate(state, target::Thread::vm_tag_dart_id());
@@ -714,6 +716,32 @@ void Assembler::TransitionNativeToGenerated(Register addr,
714716
StoreToOffset(state, THR, target::Thread::exit_through_ffi_offset());
715717
}
716718

719+
void Assembler::VerifyInGenerated(Register scratch) {
720+
#if defined(DEBUG)
721+
// Verify the thread is in generated.
722+
Comment("VerifyInGenerated");
723+
ldr(scratch, Address(THR, target::Thread::execution_state_offset()));
724+
Label ok;
725+
CompareImmediate(scratch, target::Thread::generated_execution_state());
726+
BranchIf(EQUAL, &ok, Assembler::kNearJump);
727+
Breakpoint();
728+
Bind(&ok);
729+
#endif
730+
}
731+
732+
void Assembler::VerifyNotInGenerated(Register scratch) {
733+
#if defined(DEBUG)
734+
// Verify the thread is in native or VM.
735+
Comment("VerifyNotInGenerated");
736+
ldr(scratch, Address(THR, target::Thread::execution_state_offset()));
737+
CompareImmediate(scratch, target::Thread::generated_execution_state());
738+
Label ok;
739+
BranchIf(NOT_EQUAL, &ok, Assembler::kNearJump);
740+
Breakpoint();
741+
Bind(&ok);
742+
#endif
743+
}
744+
717745
void Assembler::clrex() {
718746
int32_t encoding = (kSpecialCondition << kConditionShift) | B26 | B24 | B22 |
719747
B21 | B20 | (0xff << 12) | B4 | 0xf;

‎runtime/vm/compiler/assembler/assembler_arm.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -623,6 +623,8 @@ class Assembler : public AssemblerBase {
623623
bool exit_safepoint,
624624
bool ignore_unwind_in_progress = false,
625625
bool set_tag = true);
626+
void VerifyInGenerated(Register scratch);
627+
void VerifyNotInGenerated(Register scratch);
626628
void EnterFullSafepoint(Register scratch0, Register scratch1);
627629
void ExitFullSafepoint(Register scratch0,
628630
Register scratch1,

‎runtime/vm/compiler/assembler/assembler_arm64.cc‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1591,6 +1591,7 @@ void Assembler::TransitionGeneratedToNative(Register destination,
15911591
target::Thread::exit_through_ffi_offset());
15921592
Register tmp = new_exit_through_ffi;
15931593

1594+
VerifyInGenerated(tmp);
15941595
// Mark that the thread is executing native code.
15951596
StoreToOffset(destination, THR, target::Thread::vm_tag_offset());
15961597
LoadImmediate(tmp, target::Thread::native_execution_state());
@@ -1680,6 +1681,7 @@ void Assembler::TransitionNativeToGenerated(Register state,
16801681
#endif
16811682
}
16821683

1684+
VerifyNotInGenerated(TMP);
16831685
// Mark that the thread is executing Dart code.
16841686
if (set_tag) {
16851687
LoadImmediate(state, target::Thread::vm_tag_dart_id());
@@ -1694,6 +1696,32 @@ void Assembler::TransitionNativeToGenerated(Register state,
16941696
StoreToOffset(state, THR, target::Thread::exit_through_ffi_offset());
16951697
}
16961698

1699+
void Assembler::VerifyInGenerated(Register scratch) {
1700+
#if defined(DEBUG)
1701+
// Verify the thread is in generated.
1702+
Comment("VerifyInGenerated");
1703+
ldr(scratch, Address(THR, target::Thread::execution_state_offset()));
1704+
Label ok;
1705+
CompareImmediate(scratch, target::Thread::generated_execution_state());
1706+
BranchIf(EQUAL, &ok, Assembler::kNearJump);
1707+
Breakpoint();
1708+
Bind(&ok);
1709+
#endif
1710+
}
1711+
1712+
void Assembler::VerifyNotInGenerated(Register scratch) {
1713+
#if defined(DEBUG)
1714+
// Verify the thread is in native or VM.
1715+
Comment("VerifyNotInGenerated");
1716+
ldr(scratch, Address(THR, target::Thread::execution_state_offset()));
1717+
CompareImmediate(scratch, target::Thread::generated_execution_state());
1718+
Label ok;
1719+
BranchIf(NOT_EQUAL, &ok, Assembler::kNearJump);
1720+
Breakpoint();
1721+
Bind(&ok);
1722+
#endif
1723+
}
1724+
16971725
void Assembler::CallRuntime(const RuntimeEntry& entry,
16981726
intptr_t argument_count) {
16991727
ASSERT(!entry.is_leaf());

‎runtime/vm/compiler/assembler/assembler_arm64.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2113,6 +2113,8 @@ class Assembler : public AssemblerBase {
21132113
bool exit_safepoint,
21142114
bool ignore_unwind_in_progress = false,
21152115
bool set_tag = true);
2116+
void VerifyInGenerated(Register scratch);
2117+
void VerifyNotInGenerated(Register scratch);
21162118
void EnterFullSafepoint(Register scratch);
21172119
void ExitFullSafepoint(Register scratch, bool ignore_unwind_in_progress);
21182120

‎runtime/vm/compiler/assembler/assembler_riscv.cc‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4381,6 +4381,12 @@ void Assembler::TransitionGeneratedToNative(Register destination,
43814381
Address(THR, target::Thread::exit_through_ffi_offset()));
43824382
Register tmp = new_exit_through_ffi;
43834383

4384+
#if defined(DEBUG)
4385+
ASSERT(T2 != TMP2);
4386+
mv(T2, TMP2); // BranchIf in VerifyInGenerated clobbers TMP2.
4387+
VerifyInGenerated(tmp);
4388+
mv(TMP2, T2);
4389+
#endif
43844390
// Mark that the thread is executing native code.
43854391
sx(destination, Address(THR, target::Thread::vm_tag_offset()));
43864392
li(tmp, target::Thread::native_execution_state());
@@ -4413,6 +4419,7 @@ void Assembler::TransitionNativeToGenerated(Register state,
44134419
#endif
44144420
}
44154421

4422+
VerifyNotInGenerated(state);
44164423
// Mark that the thread is executing Dart code.
44174424
if (set_tag) {
44184425
li(state, target::Thread::vm_tag_dart_id());
@@ -4426,6 +4433,32 @@ void Assembler::TransitionNativeToGenerated(Register state,
44264433
sx(ZR, Address(THR, target::Thread::exit_through_ffi_offset()));
44274434
}
44284435

4436+
void Assembler::VerifyInGenerated(Register scratch) {
4437+
#if defined(DEBUG)
4438+
// Verify the thread is in generated.
4439+
Comment("VerifyInGenerated");
4440+
lx(scratch, Address(THR, target::Thread::execution_state_offset()));
4441+
Label ok;
4442+
CompareImmediate(scratch, target::Thread::generated_execution_state());
4443+
BranchIf(EQUAL, &ok, Assembler::kNearJump);
4444+
Breakpoint();
4445+
Bind(&ok);
4446+
#endif
4447+
}
4448+
4449+
void Assembler::VerifyNotInGenerated(Register scratch) {
4450+
#if defined(DEBUG)
4451+
// Verify the thread is in native or VM.
4452+
Comment("VerifyNotInGenerated");
4453+
lx(scratch, Address(THR, target::Thread::execution_state_offset()));
4454+
CompareImmediate(scratch, target::Thread::generated_execution_state());
4455+
Label ok;
4456+
BranchIf(NOT_EQUAL, &ok, Assembler::kNearJump);
4457+
Breakpoint();
4458+
Bind(&ok);
4459+
#endif
4460+
}
4461+
44294462
void Assembler::EnterFullSafepoint(Register state) {
44304463
// We generate the same number of instructions whether or not the slow-path is
44314464
// forced. This simplifies GenerateJitCallbackTrampolines.

‎runtime/vm/compiler/assembler/assembler_riscv.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1454,6 +1454,8 @@ class Assembler : public MicroAssembler {
14541454
bool exit_safepoint,
14551455
bool ignore_unwind_in_progress = false,
14561456
bool set_tag = true);
1457+
void VerifyInGenerated(Register scratch);
1458+
void VerifyNotInGenerated(Register scratch);
14571459
void EnterFullSafepoint(Register scratch);
14581460
void ExitFullSafepoint(Register scratch, bool ignore_unwind_in_progress);
14591461

‎runtime/vm/compiler/assembler/assembler_x64.cc‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,8 @@ void Assembler::TransitionGeneratedToNative(Register destination_address,
190190
compiler::target::Thread::exit_through_ffi_offset()),
191191
new_exit_through_ffi);
192192

193+
VerifyInGenerated(TMP);
194+
// Mark that the thread is executing native code.
193195
movq(Assembler::VMTagAddress(), destination_address);
194196
movq(Address(THR, target::Thread::execution_state_offset()),
195197
Immediate(target::Thread::native_execution_state()));
@@ -259,10 +261,10 @@ void Assembler::ExitFullSafepoint(bool ignore_unwind_in_progress) {
259261
Bind(&done);
260262
}
261263

262-
void Assembler::TransitionNativeToGenerated(bool leave_safepoint,
264+
void Assembler::TransitionNativeToGenerated(bool exit_safepoint,
263265
bool ignore_unwind_in_progress,
264266
bool set_tag) {
265-
if (leave_safepoint) {
267+
if (exit_safepoint) {
266268
ExitFullSafepoint(ignore_unwind_in_progress);
267269
} else {
268270
// flag only makes sense if we are leaving safepoint
@@ -278,6 +280,8 @@ void Assembler::TransitionNativeToGenerated(bool leave_safepoint,
278280
#endif
279281
}
280282

283+
VerifyNotInGenerated(TMP);
284+
// Mark that the thread is executing Dart code.
281285
if (set_tag) {
282286
movq(Assembler::VMTagAddress(),
283287
Immediate(target::Thread::vm_tag_dart_id()));
@@ -293,6 +297,32 @@ void Assembler::TransitionNativeToGenerated(bool leave_safepoint,
293297
compiler::Immediate(0));
294298
}
295299

300+
void Assembler::VerifyInGenerated(Register scratch) {
301+
#if defined(DEBUG)
302+
// Verify the thread is in generated.
303+
Comment("VerifyInGenerated");
304+
movq(scratch, Address(THR, target::Thread::execution_state_offset()));
305+
CompareImmediate(scratch, target::Thread::generated_execution_state());
306+
Label ok;
307+
BranchIf(EQUAL, &ok, Assembler::kNearJump);
308+
Breakpoint();
309+
Bind(&ok);
310+
#endif
311+
}
312+
313+
void Assembler::VerifyNotInGenerated(Register scratch) {
314+
#if defined(DEBUG)
315+
// Verify the thread is in native or VM.
316+
Comment("VerifyNotInGenerated");
317+
movq(scratch, Address(THR, target::Thread::execution_state_offset()));
318+
CompareImmediate(scratch, target::Thread::generated_execution_state());
319+
Label ok;
320+
BranchIf(NOT_EQUAL, &ok, Assembler::kNearJump);
321+
Breakpoint();
322+
Bind(&ok);
323+
#endif
324+
}
325+
296326
void Assembler::EmitQ(int reg,
297327
const Address& address,
298328
int opcode,

‎runtime/vm/compiler/assembler/assembler_x64.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -324,9 +324,11 @@ class Assembler : public AssemblerBase {
324324
Register new_exit_frame,
325325
Register new_exit_through_ffi,
326326
bool enter_safepoint);
327-
void TransitionNativeToGenerated(bool leave_safepoint,
327+
void TransitionNativeToGenerated(bool exit_safepoint,
328328
bool ignore_unwind_in_progress = false,
329329
bool set_tag = true);
330+
void VerifyInGenerated(Register scratch);
331+
void VerifyNotInGenerated(Register scratch);
330332

331333
// Register-register, register-address and address-register instructions.
332334
#define RR(width, name, ...) \

‎runtime/vm/compiler/backend/il_riscv.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1550,7 +1550,7 @@ void FfiCallInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
15501550
__ jalr(target);
15511551

15521552
// Update information in the thread object and leave the safepoint.
1553-
__ TransitionNativeToGenerated(temp1, /*leave_safepoint=*/true);
1553+
__ TransitionNativeToGenerated(temp1, /*exit_safepoint=*/true);
15541554
} else {
15551555
// We cannot trust that this code will be executable within a safepoint.
15561556
// Therefore we delegate the responsibility of entering/exiting the

‎runtime/vm/compiler/backend/il_x64.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1348,7 +1348,7 @@ void FfiCallInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
13481348
__ CallCFunction(target_address, /*restore_rsp=*/true);
13491349

13501350
// Update information in the thread object and leave the safepoint.
1351-
__ TransitionNativeToGenerated(/*leave_safepoint=*/true);
1351+
__ TransitionNativeToGenerated(/*exit_safepoint=*/true);
13521352
} else {
13531353
// We cannot trust that this code will be executable within a safepoint.
13541354
// Therefore we delegate the responsibility of entering/exiting the

0 commit comments

Comments
 (0)