Skip to content

Commit 06a8802

Browse files
rmacnak-googleCommit Queue
authored andcommitted
[vm, ffi] Don't return to [stub] FfiCallbackTrampoline after DLRT_ExitTemporaryIsolate.
By time we return from DLRT_ExitTemporaryIsolate, VM shutdown may have completed, including deleting the VM isolate heap containing the stub. Instead tail-call to DLRT_ExitTemporaryIsolate, so it may return directly to the foreign caller. TEST=ffi/async_void_function_callbacks_test Bug: #53248 Change-Id: I674ed672ea4a105ad6f72a86f6aedae7db1040dc Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/417963 Reviewed-by: Daco Harkes <[email protected]> Commit-Queue: Ryan Macnak <[email protected]>
1 parent 2fa4f72 commit 06a8802

File tree

5 files changed

+42
-35
lines changed

5 files changed

+42
-35
lines changed

runtime/vm/compiler/stub_code_compiler_arm.cc

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -442,15 +442,16 @@ void StubCodeCompiler::GenerateFfiCallbackTrampolineStub() {
442442

443443
// Exit the temporary isolate.
444444
{
445-
__ EnterFrame(1 << FP, 0);
446-
__ ReserveAlignedFrameSpace(0);
447-
448445
GenerateLoadFfiCallbackMetadataRuntimeFunction(
449-
FfiCallbackMetadata::kExitTemporaryIsolate, R4);
446+
FfiCallbackMetadata::kExitTemporaryIsolate, R0);
450447

451-
__ blx(R4);
448+
CLOBBERS_LR(__ PopList((1 << LR) | (1 << THR) | (1 << R4) | (1 << R5)));
452449

453-
__ LeaveFrame(1 << FP);
450+
// Tail-call DLRT_ExitTemporaryIsolate. It is not safe to return to this
451+
// stub, since it might be deleted once DLRT_ExitTemporaryIsolate proceeds
452+
// enough for VM shutdown.
453+
__ bx(R0);
454+
__ Breakpoint();
454455
}
455456

456457
__ Bind(&done);

runtime/vm/compiler/stub_code_compiler_arm64.cc

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -620,10 +620,6 @@ void StubCodeCompiler::GenerateFfiCallbackTrampolineStub() {
620620

621621
// Exit the temporary isolate.
622622
{
623-
__ SetupDartSP();
624-
__ EnterFrame(0);
625-
__ ReserveAlignedFrameSpace(0);
626-
627623
#if defined(DART_TARGET_OS_FUCHSIA)
628624
// TODO(https://dartbug.com/52579): Remove.
629625
if (FLAG_precompiled_mode) {
@@ -640,13 +636,15 @@ void StubCodeCompiler::GenerateFfiCallbackTrampolineStub() {
640636
FfiCallbackMetadata::kExitTemporaryIsolate, R4);
641637
#endif
642638

643-
__ mov(CSP, SP);
644-
__ blr(R4);
645-
__ mov(SP, CSP);
646-
__ mov(THR, R0);
639+
// Pop LR and THR from the real stack (CSP).
640+
CLOBBERS_LR(__ ldp(
641+
THR, LR, Address(CSP, 2 * target::kWordSize, Address::PairPostIndex)));
647642

648-
__ LeaveFrame();
649-
__ RestoreCSP();
643+
// Tail-call DLRT_ExitTemporaryIsolate. It is not safe to return to this
644+
// stub, since it might be deleted once DLRT_ExitTemporaryIsolate proceeds
645+
// enough for VM shutdown.
646+
__ br(R4);
647+
__ brk(0);
650648
}
651649

652650
__ Bind(&done);

runtime/vm/compiler/stub_code_compiler_ia32.cc

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -349,14 +349,20 @@ void StubCodeCompiler::GenerateFfiCallbackTrampolineStub() {
349349

350350
// Exit the temporary isolate.
351351
{
352-
__ EnterFrame(0);
353-
__ ReserveAlignedFrameSpace(0);
352+
// Pop the trampoline type into ECX.
353+
__ popl(ECX);
354+
355+
// Restore callee-saved registers.
356+
__ popl(EBX);
357+
__ popl(THR);
354358

359+
// Tail-call DLRT_ExitTemporaryIsolate. It is not safe to return to this
360+
// stub, since it might be deleted once DLRT_ExitTemporaryIsolate proceeds
361+
// enough for VM shutdown.
355362
__ movl(EAX,
356363
Immediate(reinterpret_cast<int64_t>(DLRT_ExitTemporaryIsolate)));
357-
__ CallCFunction(EAX);
358-
359-
__ LeaveFrame();
364+
__ jmp(EAX);
365+
__ int3();
360366
}
361367

362368
__ Bind(&done);

runtime/vm/compiler/stub_code_compiler_riscv.cc

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -474,16 +474,12 @@ void StubCodeCompiler::GenerateFfiCallbackTrampolineStub() {
474474

475475
// Exit the temporary isolate.
476476
{
477-
__ EnterFrame(0);
478-
__ ReserveAlignedFrameSpace(0);
479-
480-
Label call;
481-
482477
#if defined(DART_TARGET_OS_FUCHSIA)
483478
// TODO(https://dartbug.com/52579): Remove.
484479
if (FLAG_precompiled_mode) {
485480
GenerateLoadBSSEntry(BSS::Relocation::DLRT_ExitTemporaryIsolate, T1, T2);
486481
} else {
482+
Label call;
487483
const intptr_t kPCRelativeLoadOffset = 12;
488484
intptr_t start = __ CodeSize();
489485
__ auipc(T1, 0);
@@ -496,16 +492,20 @@ void StubCodeCompiler::GenerateFfiCallbackTrampolineStub() {
496492
#else
497493
__ Emit64(reinterpret_cast<int64_t>(&DLRT_ExitTemporaryIsolate));
498494
#endif
495+
__ Bind(&call);
499496
}
500497
#else
501498
GenerateLoadFfiCallbackMetadataRuntimeFunction(
502499
FfiCallbackMetadata::kExitTemporaryIsolate, T1);
503500
#endif // defined(DART_TARGET_OS_FUCHSIA)
504501

505-
__ Bind(&call);
506-
__ jalr(T1);
502+
__ PopRegisterPair(RA, THR);
507503

508-
__ LeaveFrame();
504+
// Tail-call DLRT_ExitTemporaryIsolate. It is not safe to return to this
505+
// stub, since it might be deleted once DLRT_ExitTemporaryIsolate proceeds
506+
// enough for VM shutdown.
507+
__ jr(T1);
508+
__ ebreak();
509509
}
510510

511511
__ Bind(&done);

runtime/vm/compiler/stub_code_compiler_x64.cc

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -592,12 +592,14 @@ void StubCodeCompiler::GenerateFfiCallbackTrampolineStub() {
592592
FfiCallbackMetadata::kExitTemporaryIsolate, RAX);
593593
#endif // defined(DART_TARGET_OS_FUCHSIA)
594594

595-
__ EnterFrame(0);
596-
__ ReserveAlignedFrameSpace(0);
597-
598-
__ CallCFunction(RAX);
599-
600-
__ LeaveFrame();
595+
// Restore THR (callee-saved).
596+
__ popq(THR);
597+
598+
// Tail-call DLRT_ExitTemporaryIsolate. It is not safe to return to this
599+
// stub, since it might be deleted once DLRT_ExitTemporaryIsolate proceeds
600+
// enough for VM shutdown.
601+
__ jmp(RAX);
602+
__ int3();
601603
}
602604

603605
__ Bind(&done);

0 commit comments

Comments
 (0)