Skip to content

Commit cb59df7

Browse files
rmacnak-googleCommit Queue
rmacnak-google
authored and
Commit Queue
committed
[vm, ffi] Better handle errors that are not unhandled exceptions during FFI callbacks.
Before this change, an error reaching an FFI callback would attempt to execute the normal invocation stub from the beginning in the FFI callback's frame, which quickly crashes. After this change, the runtime recognizes this marker use of the invocation stub and returns to the FFI callback function instead. TEST=ffi/unwind Bug: #39487 Change-Id: I477cfcfc236e6cf518ebfe52860ba49e466ebf8b Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/409562 Reviewed-by: Daco Harkes <[email protected]> Commit-Queue: Ryan Macnak <[email protected]>
1 parent a864586 commit cb59df7

15 files changed

+260
-60
lines changed

‎runtime/bin/ffi_test/ffi_test_functions.cc‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -928,6 +928,13 @@ DART_EXPORT intptr_t TestSimpleAddition(intptr_t (*add)(int, int)) {
928928
return 0;
929929
}
930930

931+
DART_EXPORT intptr_t TestUnwindError(intptr_t (*add)(int, int)) {
932+
const intptr_t result = add(10, 20);
933+
printf("result %" PRIdPTR "\n", result);
934+
CHECK_EQ(result, 42);
935+
return 0;
936+
}
937+
931938
//// Following tests are copied from above, with the role of Dart and C++ code
932939
//// reversed.
933940

‎runtime/bin/ffi_test/ffi_test_functions_vmspecific.cc‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1469,4 +1469,9 @@ DART_EXPORT void ManyHandles(Dart_Handle o0,
14691469
}
14701470
#undef CHECK_STRING
14711471

1472+
DART_EXPORT Dart_Handle TestUnwindErrorThroughHandle(Dart_Handle (*add)(int,
1473+
int)) {
1474+
return add(10, 20);
1475+
}
1476+
14721477
} // namespace dart

‎runtime/vm/compiler/stub_code_compiler_arm.cc‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3108,7 +3108,8 @@ void StubCodeCompiler::GenerateJumpToFrameStub() {
31083108
//
31093109
// The arguments are stored in the Thread object.
31103110
// Does not return.
3111-
void StubCodeCompiler::GenerateRunExceptionHandlerStub() {
3111+
static void GenerateRunExceptionHandler(Assembler* assembler,
3112+
bool unbox_exception) {
31123113
WRITES_RETURN_ADDRESS_TO_LR(
31133114
__ LoadFromOffset(LR, THR, target::Thread::resume_pc_offset()));
31143115

@@ -3120,6 +3121,15 @@ void StubCodeCompiler::GenerateRunExceptionHandlerStub() {
31203121
// Exception object.
31213122
__ LoadFromOffset(R0, THR, target::Thread::active_exception_offset());
31223123
__ StoreToOffset(R2, THR, target::Thread::active_exception_offset());
3124+
if (unbox_exception) {
3125+
compiler::Label not_smi, done;
3126+
__ BranchIfNotSmi(R0, &not_smi);
3127+
__ SmiUntag(R0);
3128+
__ Jump(&done);
3129+
__ Bind(&not_smi);
3130+
__ ldr(R0, FieldAddress(R0, Mint::value_offset()));
3131+
__ Bind(&done);
3132+
}
31233133

31243134
// StackTrace object.
31253135
__ LoadFromOffset(R1, THR, target::Thread::active_stacktrace_offset());
@@ -3129,6 +3139,14 @@ void StubCodeCompiler::GenerateRunExceptionHandlerStub() {
31293139
__ bx(LR)); // Jump to the exception handler code.
31303140
}
31313141

3142+
void StubCodeCompiler::GenerateRunExceptionHandlerStub() {
3143+
GenerateRunExceptionHandler(assembler, false);
3144+
}
3145+
3146+
void StubCodeCompiler::GenerateRunExceptionHandlerUnboxStub() {
3147+
GenerateRunExceptionHandler(assembler, true);
3148+
}
3149+
31323150
// Deoptimize a frame on the call stack before rewinding.
31333151
// The arguments are stored in the Thread object.
31343152
// No result.

‎runtime/vm/compiler/stub_code_compiler_arm64.cc‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3494,7 +3494,8 @@ void StubCodeCompiler::GenerateJumpToFrameStub() {
34943494
//
34953495
// The arguments are stored in the Thread object.
34963496
// Does not return.
3497-
void StubCodeCompiler::GenerateRunExceptionHandlerStub() {
3497+
static void GenerateRunExceptionHandler(Assembler* assembler,
3498+
bool unbox_exception) {
34983499
WRITES_RETURN_ADDRESS_TO_LR(
34993500
__ LoadFromOffset(LR, THR, target::Thread::resume_pc_offset()));
35003501

@@ -3506,6 +3507,15 @@ void StubCodeCompiler::GenerateRunExceptionHandlerStub() {
35063507
// Exception object.
35073508
__ LoadFromOffset(R0, THR, target::Thread::active_exception_offset());
35083509
__ StoreToOffset(R2, THR, target::Thread::active_exception_offset());
3510+
if (unbox_exception) {
3511+
compiler::Label not_smi, done;
3512+
__ BranchIfNotSmi(R0, &not_smi);
3513+
__ SmiUntag(R0);
3514+
__ Jump(&done);
3515+
__ Bind(&not_smi);
3516+
__ ldr(R0, FieldAddress(R0, Mint::value_offset()));
3517+
__ Bind(&done);
3518+
}
35093519

35103520
// StackTrace object.
35113521
__ LoadFromOffset(R1, THR, target::Thread::active_stacktrace_offset());
@@ -3514,6 +3524,14 @@ void StubCodeCompiler::GenerateRunExceptionHandlerStub() {
35143524
__ ret(); // Jump to the exception handler code.
35153525
}
35163526

3527+
void StubCodeCompiler::GenerateRunExceptionHandlerStub() {
3528+
GenerateRunExceptionHandler(assembler, false);
3529+
}
3530+
3531+
void StubCodeCompiler::GenerateRunExceptionHandlerUnboxStub() {
3532+
GenerateRunExceptionHandler(assembler, true);
3533+
}
3534+
35173535
// Deoptimize a frame on the call stack before rewinding.
35183536
// The arguments are stored in the Thread object.
35193537
// No result.

‎runtime/vm/compiler/stub_code_compiler_ia32.cc‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2987,7 +2987,8 @@ void StubCodeCompiler::GenerateJumpToFrameStub() {
29872987
//
29882988
// The arguments are stored in the Thread object.
29892989
// No result.
2990-
void StubCodeCompiler::GenerateRunExceptionHandlerStub() {
2990+
static void GenerateRunExceptionHandler(Assembler* assembler,
2991+
bool unbox_exception) {
29912992
ASSERT(kExceptionObjectReg == EAX);
29922993
ASSERT(kStackTraceObjectReg == EDX);
29932994
__ movl(EBX, Address(THR, target::Thread::resume_pc_offset()));
@@ -2999,6 +3000,17 @@ void StubCodeCompiler::GenerateRunExceptionHandlerStub() {
29993000
Address exception_addr(THR, target::Thread::active_exception_offset());
30003001
__ movl(kExceptionObjectReg, exception_addr);
30013002
__ movl(exception_addr, ECX);
3003+
if (unbox_exception) {
3004+
compiler::Label not_smi, done;
3005+
__ BranchIfNotSmi(kExceptionObjectReg, &not_smi,
3006+
compiler::Assembler::kNearJump);
3007+
__ SmiUntag(kExceptionObjectReg);
3008+
__ jmp(&done, compiler::Assembler::kNearJump);
3009+
__ Bind(&not_smi);
3010+
__ movl(kExceptionObjectReg,
3011+
compiler::FieldAddress(kExceptionObjectReg, Mint::value_offset()));
3012+
__ Bind(&done);
3013+
}
30023014

30033015
// Load the stacktrace from the current thread.
30043016
Address stacktrace_addr(THR, target::Thread::active_stacktrace_offset());
@@ -3008,6 +3020,14 @@ void StubCodeCompiler::GenerateRunExceptionHandlerStub() {
30083020
__ jmp(EBX); // Jump to continuation point.
30093021
}
30103022

3023+
void StubCodeCompiler::GenerateRunExceptionHandlerStub() {
3024+
GenerateRunExceptionHandler(assembler, false);
3025+
}
3026+
3027+
void StubCodeCompiler::GenerateRunExceptionHandlerUnboxStub() {
3028+
GenerateRunExceptionHandler(assembler, true);
3029+
}
3030+
30113031
// Deoptimize a frame on the call stack before rewinding.
30123032
// The arguments are stored in the Thread object.
30133033
// No result.

‎runtime/vm/compiler/stub_code_compiler_riscv.cc‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2967,11 +2967,21 @@ void StubCodeCompiler::GenerateJumpToFrameStub() {
29672967
//
29682968
// The arguments are stored in the Thread object.
29692969
// Does not return.
2970-
void StubCodeCompiler::GenerateRunExceptionHandlerStub() {
2970+
static void GenerateRunExceptionHandler(Assembler* assembler,
2971+
bool unbox_exception) {
29712972
// Exception object.
29722973
ASSERT(kExceptionObjectReg == A0);
29732974
__ LoadFromOffset(A0, THR, target::Thread::active_exception_offset());
29742975
__ StoreToOffset(NULL_REG, THR, target::Thread::active_exception_offset());
2976+
if (unbox_exception) {
2977+
compiler::Label not_smi, done;
2978+
__ BranchIfNotSmi(A0, &not_smi);
2979+
__ SmiUntag(A0);
2980+
__ Jump(&done);
2981+
__ Bind(&not_smi);
2982+
__ lx(A0, FieldAddress(A0, Mint::value_offset()));
2983+
__ Bind(&done);
2984+
}
29752985

29762986
// StackTrace object.
29772987
ASSERT(kStackTraceObjectReg == A1);
@@ -2982,6 +2992,14 @@ void StubCodeCompiler::GenerateRunExceptionHandlerStub() {
29822992
__ ret(); // Jump to the exception handler code.
29832993
}
29842994

2995+
void StubCodeCompiler::GenerateRunExceptionHandlerStub() {
2996+
GenerateRunExceptionHandler(assembler, false);
2997+
}
2998+
2999+
void StubCodeCompiler::GenerateRunExceptionHandlerUnboxStub() {
3000+
GenerateRunExceptionHandler(assembler, true);
3001+
}
3002+
29853003
// Deoptimize a frame on the call stack before rewinding.
29863004
// The arguments are stored in the Thread object.
29873005
// No result.

‎runtime/vm/compiler/stub_code_compiler_x64.cc‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3392,7 +3392,8 @@ void StubCodeCompiler::GenerateJumpToFrameStub() {
33923392
//
33933393
// The arguments are stored in the Thread object.
33943394
// No result.
3395-
void StubCodeCompiler::GenerateRunExceptionHandlerStub() {
3395+
static void GenerateRunExceptionHandler(Assembler* assembler,
3396+
bool unbox_exception) {
33963397
ASSERT(kExceptionObjectReg == RAX);
33973398
ASSERT(kStackTraceObjectReg == RDX);
33983399
__ movq(CallingConventions::kArg1Reg,
@@ -3407,6 +3408,17 @@ void StubCodeCompiler::GenerateRunExceptionHandlerStub() {
34073408
Address exception_addr(THR, target::Thread::active_exception_offset());
34083409
__ movq(kExceptionObjectReg, exception_addr);
34093410
__ movq(exception_addr, TMP);
3411+
if (unbox_exception) {
3412+
compiler::Label not_smi, done;
3413+
__ BranchIfNotSmi(kExceptionObjectReg, &not_smi,
3414+
compiler::Assembler::kNearJump);
3415+
__ SmiUntagAndSignExtend(kExceptionObjectReg);
3416+
__ jmp(&done, compiler::Assembler::kNearJump);
3417+
__ Bind(&not_smi);
3418+
__ movq(kExceptionObjectReg,
3419+
compiler::FieldAddress(kExceptionObjectReg, Mint::value_offset()));
3420+
__ Bind(&done);
3421+
}
34103422

34113423
// Load the stacktrace from the current thread.
34123424
Address stacktrace_addr(THR, target::Thread::active_stacktrace_offset());
@@ -3416,6 +3428,14 @@ void StubCodeCompiler::GenerateRunExceptionHandlerStub() {
34163428
__ jmp(CallingConventions::kArg1Reg); // Jump to continuation point.
34173429
}
34183430

3431+
void StubCodeCompiler::GenerateRunExceptionHandlerStub() {
3432+
GenerateRunExceptionHandler(assembler, false);
3433+
}
3434+
3435+
void StubCodeCompiler::GenerateRunExceptionHandlerUnboxStub() {
3436+
GenerateRunExceptionHandler(assembler, true);
3437+
}
3438+
34193439
// Deoptimize a frame on the call stack before rewinding.
34203440
// The arguments are stored in the Thread object.
34213441
// No result.

‎runtime/vm/exceptions.cc‎

Lines changed: 58 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include "lib/stacktrace.h"
1313

1414
#include "vm/dart_api_impl.h"
15+
#include "vm/dart_api_state.h"
1516
#include "vm/dart_entry.h"
1617
#include "vm/datastream.h"
1718
#include "vm/debugger.h"
@@ -538,24 +539,6 @@ CatchEntryMoves* CatchEntryMovesMapReader::ReadCompressedCatchEntryMovesSuffix(
538539
return moves;
539540
}
540541

541-
static void FindErrorHandler(uword* handler_pc,
542-
uword* handler_sp,
543-
uword* handler_fp) {
544-
StackFrameIterator frames(ValidationPolicy::kDontValidateFrames,
545-
Thread::Current(),
546-
StackFrameIterator::kNoCrossThreadIteration);
547-
StackFrame* frame = frames.NextFrame();
548-
ASSERT(frame != nullptr);
549-
while (!frame->IsEntryFrame()) {
550-
frame = frames.NextFrame();
551-
ASSERT(frame != nullptr);
552-
}
553-
ASSERT(frame->IsEntryFrame());
554-
*handler_pc = frame->pc();
555-
*handler_sp = frame->sp();
556-
*handler_fp = frame->fp();
557-
}
558-
559542
static void ClearLazyDeopts(Thread* thread, uword frame_pointer) {
560543
if (thread->pending_deopts().HasPendingDeopts()) {
561544
// We may be jumping over frames scheduled for lazy deopt. Remove these
@@ -590,19 +573,40 @@ static void ClearLazyDeopts(Thread* thread, uword frame_pointer) {
590573
}
591574
}
592575

576+
enum ExceptionType { kPassObject, kPassHandle, kPassUnboxed };
577+
593578
static void JumpToExceptionHandler(Thread* thread,
594579
uword program_counter,
595580
uword stack_pointer,
596581
uword frame_pointer,
597582
const Object& exception_object,
598-
const Object& stacktrace_object) {
583+
const Object& stacktrace_object,
584+
ExceptionType type = kPassObject) {
599585
bool clear_deopt = false;
600586
uword remapped_pc = thread->pending_deopts().RemapExceptionPCForDeopt(
601587
program_counter, frame_pointer, &clear_deopt);
602-
thread->set_active_exception(exception_object);
588+
uword run_exception_pc = StubCode::RunExceptionHandler().EntryPoint();
589+
switch (type) {
590+
case kPassObject:
591+
thread->set_active_exception(exception_object);
592+
break;
593+
case kPassHandle: {
594+
LocalHandle* handle =
595+
thread->api_top_scope()->local_handles()->AllocateHandle();
596+
handle->set_ptr(exception_object.ptr());
597+
thread->set_active_exception(handle);
598+
break;
599+
}
600+
case kPassUnboxed: {
601+
thread->set_active_exception(exception_object);
602+
run_exception_pc = StubCode::RunExceptionHandlerUnbox().EntryPoint();
603+
break;
604+
}
605+
default:
606+
UNREACHABLE();
607+
}
603608
thread->set_active_stacktrace(stacktrace_object);
604609
thread->set_resume_pc(remapped_pc);
605-
uword run_exception_pc = StubCode::RunExceptionHandler().EntryPoint();
606610
Exceptions::JumpToFrame(thread, run_exception_pc, stack_pointer,
607611
frame_pointer, clear_deopt);
608612
}
@@ -1012,36 +1016,43 @@ void Exceptions::PropagateError(const Error& error) {
10121016
const Instance& stk = Instance::Handle(zone, uhe.stacktrace());
10131017
Exceptions::ReThrow(thread, exc, stk);
10141018
} else {
1019+
const Instance& stk = StackTrace::Handle(zone); // Null stacktrace.
10151020
// Return to the invocation stub and return this error object. The
10161021
// C++ code which invoked this dart sequence can check and do the
10171022
// appropriate thing.
1018-
uword handler_pc = 0;
1019-
uword handler_sp = 0;
1020-
uword handler_fp = 0;
1021-
FindErrorHandler(&handler_pc, &handler_sp, &handler_fp);
1022-
JumpToExceptionHandler(thread, handler_pc, handler_sp, handler_fp, error,
1023-
StackTrace::Handle(zone)); // Null stacktrace.
1024-
}
1025-
UNREACHABLE();
1026-
}
1027-
1028-
void Exceptions::PropagateToEntry(const Error& error) {
1029-
Thread* thread = Thread::Current();
1030-
Zone* zone = thread->zone();
1031-
ASSERT(thread->top_exit_frame_info() != 0);
1032-
Instance& stacktrace = Instance::Handle(zone);
1033-
if (error.IsUnhandledException()) {
1034-
const UnhandledException& uhe = UnhandledException::Cast(error);
1035-
stacktrace = uhe.stacktrace();
1036-
} else {
1037-
stacktrace = Exceptions::CurrentStackTrace();
1023+
StackFrameIterator frames(ValidationPolicy::kDontValidateFrames, thread,
1024+
StackFrameIterator::kNoCrossThreadIteration);
1025+
StackFrame* frame = frames.NextFrame();
1026+
StackFrame* prev = frame;
1027+
ASSERT(frame != nullptr);
1028+
while (!frame->IsEntryFrame()) {
1029+
prev = frame;
1030+
frame = frames.NextFrame();
1031+
ASSERT(frame != nullptr);
1032+
}
1033+
if (frame->pc() == StubCode::InvokeDartCode().EntryPoint()) {
1034+
// This is an FFI callback using the invocation stub as a marker. Real use
1035+
// of invocation stub would be in the middle, not the entry point. Use the
1036+
// callback's exceptional return value instead of the error unless the
1037+
// return type is Dart_Handle.
1038+
ASSERT(prev->IsDartFrame());
1039+
frame = prev;
1040+
const Function& func =
1041+
Function::Handle(zone, frame->LookupDartFunction());
1042+
ASSERT(func.IsFfiCallbackTrampoline());
1043+
if (func.FfiCSignatureReturnsHandle()) {
1044+
JumpToExceptionHandler(thread, frame->pc(), frame->sp(), frame->fp(),
1045+
error, stk, kPassHandle);
1046+
} else {
1047+
const Instance& val =
1048+
Instance::Handle(zone, func.FfiCallbackExceptionalReturn());
1049+
JumpToExceptionHandler(thread, frame->pc(), frame->sp(), frame->fp(),
1050+
val, stk, kPassUnboxed);
1051+
}
1052+
}
1053+
JumpToExceptionHandler(thread, frame->pc(), frame->sp(), frame->fp(), error,
1054+
stk);
10381055
}
1039-
uword handler_pc = 0;
1040-
uword handler_sp = 0;
1041-
uword handler_fp = 0;
1042-
FindErrorHandler(&handler_pc, &handler_sp, &handler_fp);
1043-
JumpToExceptionHandler(thread, handler_pc, handler_sp, handler_fp, error,
1044-
stacktrace);
10451056
UNREACHABLE();
10461057
}
10471058

‎runtime/vm/object.cc‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8786,6 +8786,13 @@ bool Function::FfiCSignatureReturnsStruct() const {
87868786
return true;
87878787
}
87888788

8789+
bool Function::FfiCSignatureReturnsHandle() const {
8790+
ASSERT(IsFfiCallbackTrampoline());
8791+
const auto& c_signature = FunctionType::Handle(FfiCSignature());
8792+
const auto& type = AbstractType::Handle(c_signature.result_type());
8793+
return type.type_class_id() == kFfiHandleCid;
8794+
}
8795+
87898796
int32_t Function::FfiCallbackId() const {
87908797
ASSERT(IsFfiCallbackTrampoline());
87918798

‎runtime/vm/object.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3024,6 +3024,7 @@ class Function : public Object {
30243024

30253025
bool FfiCSignatureContainsHandles() const;
30263026
bool FfiCSignatureReturnsStruct() const;
3027+
bool FfiCSignatureReturnsHandle() const;
30273028

30283029
// Can only be called on FFI trampolines.
30293030
int32_t FfiCallbackId() const;

0 commit comments

Comments
 (0)