Skip to content

Commit 8085a97

Browse files
mralephCommit Queue
authored andcommitted
[vm] Fix JumpToFrame execution state transition
Instead of handling FFI related execution state and safepoint in assembly handle it in runtime code. The transition needs to be done before JumpToFrame unwinds stack because unwinding destroys exit frame and this can't be done at safepoint as GC might be traversing the stack. An incorrect order of operation was manifesting as crashes in GC when one isolate in a group was encountering a lot of exceptions thrown from an FFI call and another isolate is triggering GCs. To catch this in the future added a bit of validation to ExitSafepoint runtime call which triggers when --use-slow-path is enabled. Though after refactoring this code does not trigger this code path anymore because it was completely removed - but it is better than nothing. This CL also removes a lot of unnecessary complexity which was associated with handling this transition in the stub itself. TEST=ffi/vmspecific_handle_test Bug: b/408377905 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-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-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-eager-optimization-linux-release-ia32-try,vm-eager-optimization-linux-release-x64-try,vm-ffi-android-debug-arm-try,vm-ffi-android-debug-arm64c-try,vm-ffi-mac-debug-simarm64_arm64-try,vm-ffi-qemu-linux-release-arm-try,vm-ffi-qemu-linux-release-riscv64-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-x64-try,vm-win-debug-x64c-try,vm-win-release-ia32-try Change-Id: Ia073cb6bb9e1b5a0ea8514c7e048cee6019b84d6 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/420324 Commit-Queue: Slava Egorov <vegorov@google.com> Reviewed-by: Daco Harkes <dacoharkes@google.com>
1 parent d33c6c3 commit 8085a97

33 files changed

+2230
-2450
lines changed

runtime/vm/compiler/assembler/assembler_arm.cc

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -634,9 +634,7 @@ void Assembler::TransitionGeneratedToNative(Register destination_address,
634634
}
635635
}
636636

637-
void Assembler::ExitFullSafepoint(Register tmp1,
638-
Register tmp2,
639-
bool ignore_unwind_in_progress) {
637+
void Assembler::ExitFullSafepoint(Register tmp1, Register tmp2) {
640638
Register addr = tmp1;
641639
Register state = tmp2;
642640

@@ -664,14 +662,7 @@ void Assembler::ExitFullSafepoint(Register tmp1,
664662
}
665663

666664
Bind(&slow_path);
667-
if (ignore_unwind_in_progress) {
668-
ldr(TMP,
669-
Address(THR,
670-
target::Thread::
671-
exit_safepoint_ignore_unwind_in_progress_stub_offset()));
672-
} else {
673-
ldr(TMP, Address(THR, target::Thread::exit_safepoint_stub_offset()));
674-
}
665+
ldr(TMP, Address(THR, target::Thread::exit_safepoint_stub_offset()));
675666
ldr(TMP, FieldAddress(TMP, target::Code::entry_point_offset()));
676667
blx(TMP);
677668

@@ -681,13 +672,10 @@ void Assembler::ExitFullSafepoint(Register tmp1,
681672
void Assembler::TransitionNativeToGenerated(Register addr,
682673
Register state,
683674
bool exit_safepoint,
684-
bool ignore_unwind_in_progress,
685675
bool set_tag) {
686676
if (exit_safepoint) {
687-
ExitFullSafepoint(addr, state, ignore_unwind_in_progress);
677+
ExitFullSafepoint(addr, state);
688678
} else {
689-
// flag only makes sense if we are leaving safepoint
690-
ASSERT(!ignore_unwind_in_progress);
691679
#if defined(DEBUG)
692680
// Ensure we've already left the safepoint.
693681
ASSERT(target::Thread::native_safepoint_state_acquired() != 0);

runtime/vm/compiler/assembler/assembler_arm.h

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -621,14 +621,11 @@ class Assembler : public AssemblerBase {
621621
void TransitionNativeToGenerated(Register scratch0,
622622
Register scratch1,
623623
bool exit_safepoint,
624-
bool ignore_unwind_in_progress = false,
625624
bool set_tag = true);
626625
void VerifyInGenerated(Register scratch);
627626
void VerifyNotInGenerated(Register scratch);
628627
void EnterFullSafepoint(Register scratch0, Register scratch1);
629-
void ExitFullSafepoint(Register scratch0,
630-
Register scratch1,
631-
bool ignore_unwind_in_progress);
628+
void ExitFullSafepoint(Register scratch0, Register scratch1);
632629

633630
// Miscellaneous instructions.
634631
void clrex();

runtime/vm/compiler/assembler/assembler_arm64.cc

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1619,8 +1619,7 @@ void Assembler::TransitionGeneratedToNative(Register destination,
16191619
}
16201620
}
16211621

1622-
void Assembler::ExitFullSafepoint(Register state,
1623-
bool ignore_unwind_in_progress) {
1622+
void Assembler::ExitFullSafepoint(Register state) {
16241623
// We generate the same number of instructions whether or not the slow-path is
16251624
// forced, for consistency with EnterFullSafepoint.
16261625
// For TSAN, we always go to the runtime so TSAN is aware of the acquire
@@ -1650,14 +1649,7 @@ void Assembler::ExitFullSafepoint(Register state,
16501649
}
16511650

16521651
Bind(&slow_path);
1653-
if (ignore_unwind_in_progress) {
1654-
ldr(addr,
1655-
Address(THR,
1656-
target::Thread::
1657-
exit_safepoint_ignore_unwind_in_progress_stub_offset()));
1658-
} else {
1659-
ldr(addr, Address(THR, target::Thread::exit_safepoint_stub_offset()));
1660-
}
1652+
ldr(addr, Address(THR, target::Thread::exit_safepoint_stub_offset()));
16611653
ldr(addr, FieldAddress(addr, target::Code::entry_point_offset()));
16621654
blr(addr);
16631655

@@ -1666,13 +1658,10 @@ void Assembler::ExitFullSafepoint(Register state,
16661658

16671659
void Assembler::TransitionNativeToGenerated(Register state,
16681660
bool exit_safepoint,
1669-
bool ignore_unwind_in_progress,
16701661
bool set_tag) {
16711662
if (exit_safepoint) {
1672-
ExitFullSafepoint(state, ignore_unwind_in_progress);
1663+
ExitFullSafepoint(state);
16731664
} else {
1674-
// flag only makes sense if we are leaving safepoint
1675-
ASSERT(!ignore_unwind_in_progress);
16761665
#if defined(DEBUG)
16771666
// Ensure we've already left the safepoint.
16781667
ASSERT(target::Thread::native_safepoint_state_acquired() != 0);

runtime/vm/compiler/assembler/assembler_arm64.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2111,12 +2111,11 @@ class Assembler : public AssemblerBase {
21112111
bool enter_safepoint);
21122112
void TransitionNativeToGenerated(Register scratch,
21132113
bool exit_safepoint,
2114-
bool ignore_unwind_in_progress = false,
21152114
bool set_tag = true);
21162115
void VerifyInGenerated(Register scratch);
21172116
void VerifyNotInGenerated(Register scratch);
21182117
void EnterFullSafepoint(Register scratch);
2119-
void ExitFullSafepoint(Register scratch, bool ignore_unwind_in_progress);
2118+
void ExitFullSafepoint(Register scratch);
21202119

21212120
void CheckCodePointer();
21222121
void RestoreCodePointer();

runtime/vm/compiler/assembler/assembler_ia32.cc

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2537,8 +2537,7 @@ void Assembler::TransitionGeneratedToNative(Register destination_address,
25372537
}
25382538
}
25392539

2540-
void Assembler::ExitFullSafepoint(Register scratch,
2541-
bool ignore_unwind_in_progress) {
2540+
void Assembler::ExitFullSafepoint(Register scratch) {
25422541
ASSERT(scratch != EAX);
25432542
// We generate the same number of instructions whether or not the slow-path is
25442543
// forced, for consistency with EnterFullSafepoint.
@@ -2563,14 +2562,7 @@ void Assembler::ExitFullSafepoint(Register scratch,
25632562
}
25642563

25652564
Bind(&slow_path);
2566-
if (ignore_unwind_in_progress) {
2567-
movl(scratch,
2568-
Address(THR,
2569-
target::Thread::
2570-
exit_safepoint_ignore_unwind_in_progress_stub_offset()));
2571-
} else {
2572-
movl(scratch, Address(THR, target::Thread::exit_safepoint_stub_offset()));
2573-
}
2565+
movl(scratch, Address(THR, target::Thread::exit_safepoint_stub_offset()));
25742566
movl(scratch, FieldAddress(scratch, target::Code::entry_point_offset()));
25752567
call(scratch);
25762568

@@ -2579,13 +2571,10 @@ void Assembler::ExitFullSafepoint(Register scratch,
25792571

25802572
void Assembler::TransitionNativeToGenerated(Register scratch,
25812573
bool exit_safepoint,
2582-
bool ignore_unwind_in_progress,
25832574
bool set_tag) {
25842575
if (exit_safepoint) {
2585-
ExitFullSafepoint(scratch, ignore_unwind_in_progress);
2576+
ExitFullSafepoint(scratch);
25862577
} else {
2587-
// flag only makes sense if we are leaving safepoint
2588-
ASSERT(!ignore_unwind_in_progress);
25892578
#if defined(DEBUG)
25902579
// Ensure we've already left the safepoint.
25912580
movl(scratch, Address(THR, target::Thread::safepoint_state_offset()));

runtime/vm/compiler/assembler/assembler_ia32.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -901,10 +901,9 @@ class Assembler : public AssemblerBase {
901901
bool enter_safepoint);
902902
void TransitionNativeToGenerated(Register scratch,
903903
bool exit_safepoint,
904-
bool ignore_unwind_in_progress = false,
905904
bool set_tag = true);
906905
void EnterFullSafepoint(Register scratch);
907-
void ExitFullSafepoint(Register scratch, bool ignore_unwind_in_progress);
906+
void ExitFullSafepoint(Register scratch);
908907

909908
// For non-leaf runtime calls. For leaf runtime calls, use LeafRuntimeScope,
910909
void CallRuntime(const RuntimeEntry& entry, intptr_t argument_count);

runtime/vm/compiler/assembler/assembler_riscv.cc

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4399,13 +4399,10 @@ void Assembler::TransitionGeneratedToNative(Register destination,
43994399

44004400
void Assembler::TransitionNativeToGenerated(Register state,
44014401
bool exit_safepoint,
4402-
bool ignore_unwind_in_progress,
44034402
bool set_tag) {
44044403
if (exit_safepoint) {
4405-
ExitFullSafepoint(state, ignore_unwind_in_progress);
4404+
ExitFullSafepoint(state);
44064405
} else {
4407-
// flag only makes sense if we are leaving safepoint
4408-
ASSERT(!ignore_unwind_in_progress);
44094406
#if defined(DEBUG)
44104407
// Ensure we've already left the safepoint.
44114408
ASSERT(target::Thread::native_safepoint_state_acquired() != 0);
@@ -4495,8 +4492,7 @@ void Assembler::EnterFullSafepoint(Register state) {
44954492
Bind(&done);
44964493
}
44974494

4498-
void Assembler::ExitFullSafepoint(Register state,
4499-
bool ignore_unwind_in_progress) {
4495+
void Assembler::ExitFullSafepoint(Register state) {
45004496
// We generate the same number of instructions whether or not the slow-path is
45014497
// forced, for consistency with EnterFullSafepoint.
45024498
// For TSAN, we always go to the runtime so TSAN is aware of the acquire
@@ -4524,14 +4520,7 @@ void Assembler::ExitFullSafepoint(Register state,
45244520
}
45254521

45264522
Bind(&slow_path);
4527-
if (ignore_unwind_in_progress) {
4528-
lx(addr,
4529-
Address(THR,
4530-
target::Thread::
4531-
exit_safepoint_ignore_unwind_in_progress_stub_offset()));
4532-
} else {
4533-
lx(addr, Address(THR, target::Thread::exit_safepoint_stub_offset()));
4534-
}
4523+
lx(addr, Address(THR, target::Thread::exit_safepoint_stub_offset()));
45354524
lx(addr, FieldAddress(addr, target::Code::entry_point_offset()));
45364525
jalr(addr);
45374526

runtime/vm/compiler/assembler/assembler_riscv.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1452,12 +1452,11 @@ class Assembler : public MicroAssembler {
14521452
bool enter_safepoint);
14531453
void TransitionNativeToGenerated(Register scratch,
14541454
bool exit_safepoint,
1455-
bool ignore_unwind_in_progress = false,
14561455
bool set_tag = true);
14571456
void VerifyInGenerated(Register scratch);
14581457
void VerifyNotInGenerated(Register scratch);
14591458
void EnterFullSafepoint(Register scratch);
1460-
void ExitFullSafepoint(Register scratch, bool ignore_unwind_in_progress);
1459+
void ExitFullSafepoint(Register scratch);
14611460

14621461
void CheckFpSpDist(intptr_t fp_sp_dist);
14631462

runtime/vm/compiler/assembler/assembler_x64.cc

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ void Assembler::TransitionGeneratedToNative(Register destination_address,
216216
}
217217
}
218218

219-
void Assembler::ExitFullSafepoint(bool ignore_unwind_in_progress) {
219+
void Assembler::ExitFullSafepoint() {
220220
// We generate the same number of instructions whether or not the slow-path is
221221
// forced, for consistency with EnterFullSafepoint.
222222
// For TSAN, we always go to the runtime so TSAN is aware of the acquire
@@ -243,14 +243,7 @@ void Assembler::ExitFullSafepoint(bool ignore_unwind_in_progress) {
243243
}
244244

245245
Bind(&slow_path);
246-
if (ignore_unwind_in_progress) {
247-
movq(TMP,
248-
Address(THR,
249-
target::Thread::
250-
exit_safepoint_ignore_unwind_in_progress_stub_offset()));
251-
} else {
252-
movq(TMP, Address(THR, target::Thread::exit_safepoint_stub_offset()));
253-
}
246+
movq(TMP, Address(THR, target::Thread::exit_safepoint_stub_offset()));
254247
movq(TMP, FieldAddress(TMP, target::Code::entry_point_offset()));
255248

256249
// Use call instead of CallCFunction to avoid having to clean up shadow space
@@ -261,14 +254,10 @@ void Assembler::ExitFullSafepoint(bool ignore_unwind_in_progress) {
261254
Bind(&done);
262255
}
263256

264-
void Assembler::TransitionNativeToGenerated(bool exit_safepoint,
265-
bool ignore_unwind_in_progress,
266-
bool set_tag) {
257+
void Assembler::TransitionNativeToGenerated(bool exit_safepoint, bool set_tag) {
267258
if (exit_safepoint) {
268-
ExitFullSafepoint(ignore_unwind_in_progress);
259+
ExitFullSafepoint();
269260
} else {
270-
// flag only makes sense if we are leaving safepoint
271-
ASSERT(!ignore_unwind_in_progress);
272261
#if defined(DEBUG)
273262
// Ensure we've already left the safepoint.
274263
movq(TMP, Address(THR, target::Thread::safepoint_state_offset()));

runtime/vm/compiler/assembler/assembler_x64.h

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -319,14 +319,12 @@ class Assembler : public AssemblerBase {
319319
void setcc(Condition condition, ByteRegister dst);
320320

321321
void EnterFullSafepoint();
322-
void ExitFullSafepoint(bool ignore_unwind_in_progress);
322+
void ExitFullSafepoint();
323323
void TransitionGeneratedToNative(Register destination_address,
324324
Register new_exit_frame,
325325
Register new_exit_through_ffi,
326326
bool enter_safepoint);
327-
void TransitionNativeToGenerated(bool exit_safepoint,
328-
bool ignore_unwind_in_progress = false,
329-
bool set_tag = true);
327+
void TransitionNativeToGenerated(bool exit_safepoint, bool set_tag = true);
330328
void VerifyInGenerated(Register scratch);
331329
void VerifyNotInGenerated(Register scratch);
332330

0 commit comments

Comments
 (0)