Skip to content

Commit 7b42c0b

Browse files
isheludkoV8 LUCI CQ
isheludko
authored and
V8 LUCI CQ
committed
[builtins] Port HandleApiCall to CSA
... in order to let it access the caller JS frame directly to ease the caller context computation (will be implemented in a follow-up CL). Additional changes related to CallApiCallbackGeneric: 1) introduce ApiCallbackExit frame so that Api functions called via CallApiCallbackGeneric could appear in the exception stack traces and to allow CallApiCallbackGeneric perform callback side effects checking when necessary, 2) add reference from CallHandlerInfo to FunctionTemplateInfo or ObjectTemplateInfo, so that CallApiCallbackGeneric could find the respective "function" object for side effects checking, 3) remove CSA::IsSideEffectFreeDebuggingActive() because CallApiCallbackGeneric is now able to handle side effects checking. Bug: v8:13825 Change-Id: I5a96051c60a8b361e27077ff76103ad93e599843 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4582948 Reviewed-by: Marja Hölttä <[email protected]> Commit-Queue: Igor Sheludko <[email protected]> Cr-Commit-Position: refs/heads/main@{#88141}
1 parent e0407b1 commit 7b42c0b

32 files changed

Lines changed: 522 additions & 161 deletions

‎src/api/api.cc‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1615,6 +1615,7 @@ void FunctionTemplate::SetCallHandler(
16151615
i::HandleScope scope(i_isolate);
16161616
i::Handle<i::CallHandlerInfo> obj = i_isolate->factory()->NewCallHandlerInfo(
16171617
side_effect_type == SideEffectType::kHasNoSideEffect);
1618+
obj->set_owner_template(*info);
16181619
obj->set_callback(i_isolate, reinterpret_cast<i::Address>(callback));
16191620
if (data.IsEmpty()) {
16201621
data = v8::Undefined(reinterpret_cast<v8::Isolate*>(i_isolate));
@@ -2066,6 +2067,7 @@ void ObjectTemplate::SetCallAsFunctionHandler(FunctionCallback callback,
20662067
EnsureNotPublished(cons, "v8::ObjectTemplate::SetCallAsFunctionHandler");
20672068
i::Handle<i::CallHandlerInfo> obj =
20682069
i_isolate->factory()->NewCallHandlerInfo();
2070+
obj->set_owner_template(*Utils::OpenHandle(this));
20692071
obj->set_callback(i_isolate, reinterpret_cast<i::Address>(callback));
20702072
if (data.IsEmpty()) {
20712073
data = v8::Undefined(reinterpret_cast<v8::Isolate*>(i_isolate));
@@ -11390,8 +11392,22 @@ inline void InvokeFunctionCallback(
1139011392

1139111393
switch (mode) {
1139211394
case CallApiCallbackMode::kGeneric: {
11393-
// TODO(v8:13825): perform side effect checks if necessary once
11394-
// CallHandlerInfo is passed here.
11395+
if (V8_UNLIKELY(i_isolate->should_check_side_effects())) {
11396+
// Load FunctionTemplateInfo from API_CALLBACK_EXIT frame.
11397+
// If this ever becomes a performance bottleneck, one can pass function
11398+
// template info here explicitly.
11399+
StackFrameIterator it(i_isolate);
11400+
CHECK(it.frame()->is_api_callback_exit());
11401+
ApiCallbackExitFrame* frame = ApiCallbackExitFrame::cast(it.frame());
11402+
FunctionTemplateInfo fti = FunctionTemplateInfo::cast(frame->target());
11403+
CallHandlerInfo call_handler_info =
11404+
CallHandlerInfo::cast(fti.call_code(kAcquireLoad));
11405+
if (!i_isolate->debug()->PerformSideEffectCheckForCallback(
11406+
handle(call_handler_info, i_isolate))) {
11407+
// Failed side effect check.
11408+
return;
11409+
}
11410+
}
1139511411
break;
1139611412
}
1139711413
case CallApiCallbackMode::kWithSideEffects: {

‎src/builtins/arm/builtins-arm.cc‎

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3259,9 +3259,6 @@ void Builtins::Generate_CallApiCallbackImpl(MacroAssembler* masm,
32593259
case CallApiCallbackMode::kGeneric:
32603260
__ ldr(scratch2, FieldMemOperand(callback, CallHandlerInfo::kDataOffset));
32613261
__ str(scratch2, MemOperand(sp, FCA::kDataIndex * kSystemPointerSize));
3262-
__ ldr(api_function_address,
3263-
FieldMemOperand(callback,
3264-
CallHandlerInfo::kMaybeRedirectedCallbackOffset));
32653262
break;
32663263

32673264
case CallApiCallbackMode::kNoSideEffects:
@@ -3286,9 +3283,35 @@ void Builtins::Generate_CallApiCallbackImpl(MacroAssembler* masm,
32863283
static_assert(FCI::kImplicitArgsOffset == 0);
32873284
static_assert(FCI::kValuesOffset == 1 * kPointerSize);
32883285
static_assert(FCI::kLengthOffset == 2 * kPointerSize);
3286+
const int exit_frame_params_size =
3287+
mode == CallApiCallbackMode::kGeneric ? 2 : 0;
32893288

32903289
FrameScope frame_scope(masm, StackFrame::MANUAL);
3291-
__ EnterExitFrame(kApiStackSpace, StackFrame::EXIT);
3290+
if (mode == CallApiCallbackMode::kGeneric) {
3291+
ASM_CODE_COMMENT_STRING(masm, "Push API_CALLBACK_EXIT frame arguments");
3292+
__ AllocateStackSpace(exit_frame_params_size * kSystemPointerSize);
3293+
3294+
// Argc parameter as a Smi.
3295+
static_assert(ApiCallbackExitFrameConstants::kArgcOffset ==
3296+
3 * kSystemPointerSize);
3297+
__ SmiTag(scratch, argc);
3298+
__ str(scratch, MemOperand(sp, 1 * kSystemPointerSize));
3299+
3300+
// Target parameter.
3301+
static_assert(ApiCallbackExitFrameConstants::kTargetOffset ==
3302+
2 * kSystemPointerSize);
3303+
__ ldr(scratch,
3304+
FieldMemOperand(callback, CallHandlerInfo::kOwnerTemplateOffset));
3305+
__ str(scratch, MemOperand(sp, 0 * kSystemPointerSize));
3306+
3307+
__ ldr(api_function_address,
3308+
FieldMemOperand(callback,
3309+
CallHandlerInfo::kMaybeRedirectedCallbackOffset));
3310+
3311+
__ EnterExitFrame(kApiStackSpace, StackFrame::API_CALLBACK_EXIT);
3312+
} else {
3313+
__ EnterExitFrame(kApiStackSpace, StackFrame::EXIT);
3314+
}
32923315

32933316
{
32943317
ASM_CODE_COMMENT_STRING(masm, "Initialize FunctionCallbackInfo");
@@ -3310,8 +3333,9 @@ void Builtins::Generate_CallApiCallbackImpl(MacroAssembler* masm,
33103333
// from the API function here.
33113334
MemOperand stack_space_operand =
33123335
ExitFrameStackSlotOperand(FCI::kLengthOffset + kSlotsToDropOnStackSize);
3313-
__ mov(scratch,
3314-
Operand((FCA::kArgsLength + 1 /* receiver */) * kPointerSize));
3336+
__ mov(scratch, Operand((FCA::kArgsLength + 1 /* receiver */ +
3337+
exit_frame_params_size) *
3338+
kPointerSize));
33153339
__ add(scratch, scratch, Operand(argc, LSL, kPointerSizeLog2));
33163340
__ str(scratch, stack_space_operand);
33173341

@@ -3326,8 +3350,8 @@ void Builtins::Generate_CallApiCallbackImpl(MacroAssembler* masm,
33263350
// checking is enabled.
33273351
Register thunk_arg = api_function_address;
33283352

3329-
MemOperand return_value_operand =
3330-
ExitFrameCallerStackSlotOperand(FCA::kReturnValueIndex);
3353+
MemOperand return_value_operand = ExitFrameCallerStackSlotOperand(
3354+
FCA::kReturnValueIndex + exit_frame_params_size);
33313355
static constexpr int kUseStackSpaceOperand = 0;
33323356

33333357
CallApiFunctionAndReturn(masm, api_function_address, thunk_ref, thunk_arg,

‎src/builtins/arm64/builtins-arm64.cc‎

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5381,11 +5381,6 @@ void Builtins::Generate_CallApiCallbackImpl(MacroAssembler* masm,
53815381
__ LoadTaggedField(
53825382
scratch2, FieldMemOperand(callback, CallHandlerInfo::kDataOffset));
53835383
__ Str(scratch2, MemOperand(sp, FCA::kDataIndex * kSystemPointerSize));
5384-
__ LoadExternalPointerField(
5385-
api_function_address,
5386-
FieldMemOperand(callback,
5387-
CallHandlerInfo::kMaybeRedirectedCallbackOffset),
5388-
kCallHandlerInfoCallbackTag);
53895384
break;
53905385

53915386
case CallApiCallbackMode::kNoSideEffects:
@@ -5410,9 +5405,38 @@ void Builtins::Generate_CallApiCallbackImpl(MacroAssembler* masm,
54105405
static_assert(FCI::kImplicitArgsOffset == 0);
54115406
static_assert(FCI::kValuesOffset == 1 * kSystemPointerSize);
54125407
static_assert(FCI::kLengthOffset == 2 * kSystemPointerSize);
5408+
const int exit_frame_params_size =
5409+
mode == CallApiCallbackMode::kGeneric ? 2 : 0;
54135410

54145411
FrameScope frame_scope(masm, StackFrame::MANUAL);
5415-
__ EnterExitFrame(scratch, kApiStackSpace, StackFrame::EXIT);
5412+
if (mode == CallApiCallbackMode::kGeneric) {
5413+
ASM_CODE_COMMENT_STRING(masm, "Push API_CALLBACK_EXIT frame arguments");
5414+
__ Claim(exit_frame_params_size, kSystemPointerSize);
5415+
5416+
// Argc parameter as a Smi.
5417+
static_assert(ApiCallbackExitFrameConstants::kArgcOffset ==
5418+
3 * kSystemPointerSize);
5419+
__ SmiTag(scratch, argc);
5420+
__ Str(scratch, MemOperand(sp, 1 * kSystemPointerSize));
5421+
5422+
// Target parameter.
5423+
static_assert(ApiCallbackExitFrameConstants::kTargetOffset ==
5424+
2 * kSystemPointerSize);
5425+
__ LoadTaggedField(
5426+
scratch,
5427+
FieldMemOperand(callback, CallHandlerInfo::kOwnerTemplateOffset));
5428+
__ Str(scratch, MemOperand(sp, 0 * kSystemPointerSize));
5429+
5430+
__ LoadExternalPointerField(
5431+
api_function_address,
5432+
FieldMemOperand(callback,
5433+
CallHandlerInfo::kMaybeRedirectedCallbackOffset),
5434+
kCallHandlerInfoCallbackTag);
5435+
5436+
__ EnterExitFrame(scratch, kApiStackSpace, StackFrame::API_CALLBACK_EXIT);
5437+
} else {
5438+
__ EnterExitFrame(scratch, kApiStackSpace, StackFrame::EXIT);
5439+
}
54165440

54175441
{
54185442
ASM_CODE_COMMENT_STRING(masm, "Initialize FunctionCallbackInfo");
@@ -5438,7 +5462,8 @@ void Builtins::Generate_CallApiCallbackImpl(MacroAssembler* masm,
54385462
// register containing the slot count.
54395463
MemOperand stack_space_operand =
54405464
ExitFrameStackSlotOperand(FCI::kLengthOffset + kSlotsToDropOnStackSize);
5441-
__ Add(scratch, argc, Operand(FCA::kArgsLengthWithReceiver));
5465+
__ Add(scratch, argc,
5466+
Operand(FCA::kArgsLengthWithReceiver + exit_frame_params_size));
54425467
__ Str(scratch, stack_space_operand);
54435468

54445469
__ RecordComment("v8::FunctionCallback's argument.");
@@ -5456,8 +5481,8 @@ void Builtins::Generate_CallApiCallbackImpl(MacroAssembler* masm,
54565481
// The current frame needs to be aligned.
54575482
DCHECK_EQ(FCA::kArgsLength % 2, 0);
54585483

5459-
MemOperand return_value_operand =
5460-
ExitFrameCallerStackSlotOperand(FCA::kReturnValueIndex);
5484+
MemOperand return_value_operand = ExitFrameCallerStackSlotOperand(
5485+
FCA::kReturnValueIndex + exit_frame_params_size);
54615486
static constexpr int kUseStackSpaceOperand = 0;
54625487

54635488
CallApiFunctionAndReturn(masm, api_function_address, thunk_ref, thunk_arg,

‎src/builtins/builtins-api.cc‎

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -132,23 +132,18 @@ V8_WARN_UNUSED_RESULT MaybeHandle<Object> HandleApiCallHelper(
132132

133133
} // anonymous namespace
134134

135-
BUILTIN(HandleApiCall) {
135+
BUILTIN(HandleApiConstruct) {
136136
HandleScope scope(isolate);
137137
Handle<Object> receiver = args.receiver();
138138
Handle<HeapObject> new_target = args.new_target();
139+
DCHECK(!new_target->IsUndefined(isolate));
139140
Handle<FunctionTemplateInfo> fun_data(
140141
args.target()->shared().get_api_func_data(), isolate);
141142
int argc = args.length() - 1;
142143
Address* argv = args.address_of_first_argument();
143-
if (new_target->IsUndefined()) {
144-
RETURN_RESULT_OR_FAILURE(
145-
isolate, HandleApiCallHelper<false>(isolate, new_target, fun_data,
146-
receiver, argv, argc));
147-
} else {
148-
RETURN_RESULT_OR_FAILURE(
149-
isolate, HandleApiCallHelper<true>(isolate, new_target, fun_data,
150-
receiver, argv, argc));
151-
}
144+
RETURN_RESULT_OR_FAILURE(
145+
isolate, HandleApiCallHelper<true>(isolate, new_target, fun_data,
146+
receiver, argv, argc));
152147
}
153148

154149
namespace {
@@ -210,8 +205,10 @@ MaybeHandle<Object> Builtins::InvokeApiFunction(
210205
// Helper function to handle calls to non-function objects created through the
211206
// API. The object can be called as either a constructor (using new) or just as
212207
// a function (without new).
213-
V8_WARN_UNUSED_RESULT static Object HandleApiCallAsFunctionOrConstructor(
214-
Isolate* isolate, bool is_construct_call, BuiltinArguments args) {
208+
V8_WARN_UNUSED_RESULT static Object
209+
HandleApiCallAsFunctionOrConstructorDelegate(Isolate* isolate,
210+
bool is_construct_call,
211+
BuiltinArguments args) {
215212
Handle<Object> receiver = args.receiver();
216213

217214
// Get the object called.
@@ -260,14 +257,14 @@ V8_WARN_UNUSED_RESULT static Object HandleApiCallAsFunctionOrConstructor(
260257

261258
// Handle calls to non-function objects created through the API. This delegate
262259
// function is used when the call is a normal function call.
263-
BUILTIN(HandleApiCallAsFunction) {
264-
return HandleApiCallAsFunctionOrConstructor(isolate, false, args);
260+
BUILTIN(HandleApiCallAsFunctionDelegate) {
261+
return HandleApiCallAsFunctionOrConstructorDelegate(isolate, false, args);
265262
}
266263

267264
// Handle calls to non-function objects created through the API. This delegate
268265
// function is used when the call is a construct call.
269-
BUILTIN(HandleApiCallAsConstructor) {
270-
return HandleApiCallAsFunctionOrConstructor(isolate, true, args);
266+
BUILTIN(HandleApiCallAsConstructorDelegate) {
267+
return HandleApiCallAsFunctionOrConstructorDelegate(isolate, true, args);
271268
}
272269

273270
} // namespace internal

‎src/builtins/builtins-call-gen.cc‎

Lines changed: 50 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -714,8 +714,11 @@ void CallOrConstructBuiltinsAssembler::CallFunctionTemplate(
714714

715715
BIND(&receiver_needs_access_check);
716716
{
717-
CallRuntime(Runtime::kAccessCheck, context, receiver);
718-
Goto(&receiver_done);
717+
TNode<BoolT> has_access =
718+
IsTrue(CallRuntime(Runtime::kAccessCheck, context, receiver));
719+
GotoIf(has_access, &receiver_done);
720+
// Access check failed, return undefined value.
721+
args.PopAndReturn(UndefinedConstant());
719722
}
720723

721724
BIND(&receiver_done);
@@ -743,9 +746,21 @@ void CallOrConstructBuiltinsAssembler::CallFunctionTemplate(
743746
[&]() { return GetCompatibleReceiver(receiver, signature, context); });
744747
}
745748

749+
TNode<HeapObject> call_code = CAST(LoadObjectField(
750+
function_template_info, FunctionTemplateInfo::kCallCodeOffset));
751+
// If the function doesn't have an associated C++ code to execute, just
752+
// return the receiver as would an empty function do (see
753+
// HandleApiCallHelper).
754+
{
755+
Label if_continue(this);
756+
GotoIfNot(IsUndefined(call_code), &if_continue);
757+
args.PopAndReturn(receiver);
758+
759+
Bind(&if_continue);
760+
}
761+
746762
// Perform the actual API callback invocation via CallApiCallback.
747-
TNode<CallHandlerInfo> call_handler_info = LoadObjectField<CallHandlerInfo>(
748-
function_template_info, FunctionTemplateInfo::kCallCodeOffset);
763+
TNode<CallHandlerInfo> call_handler_info = CAST(call_code);
749764
TailCallStub(
750765
Builtins::CallableFor(isolate(), Builtin::kCallApiCallbackGeneric),
751766
context, TruncateIntPtrToInt32(args.GetLengthWithoutReceiver()),
@@ -782,5 +797,36 @@ TF_BUILTIN(CallFunctionTemplate_CheckAccessAndCompatibleReceiver,
782797
function_template_info, argc, context);
783798
}
784799

800+
TF_BUILTIN(HandleApiCallOrConstruct, CallOrConstructBuiltinsAssembler) {
801+
auto target = Parameter<Object>(Descriptor::kJSTarget);
802+
auto new_target = Parameter<Object>(Descriptor::kJSNewTarget);
803+
auto context = Parameter<Context>(Descriptor::kContext);
804+
auto argc = UncheckedParameter<Int32T>(Descriptor::kJSActualArgumentsCount);
805+
806+
Label if_call(this), if_construct(this);
807+
Branch(IsUndefined(new_target), &if_call, &if_construct);
808+
809+
BIND(&if_call);
810+
{
811+
TNode<SharedFunctionInfo> shared =
812+
LoadJSFunctionSharedFunctionInfo(CAST(target));
813+
TNode<FunctionTemplateInfo> function_template_info =
814+
CAST(LoadSharedFunctionInfoFunctionData(shared));
815+
816+
// Tail call to the stub while leaving all the incoming JS arguments on
817+
// the stack.
818+
TailCallBuiltin(
819+
Builtin::kCallFunctionTemplate_CheckAccessAndCompatibleReceiver,
820+
context, function_template_info, ChangeUint32ToWord(argc));
821+
}
822+
BIND(&if_construct);
823+
{
824+
// Tail call to the stub while leaving all the incoming JS arguments on
825+
// the stack.
826+
TailCallBuiltin(Builtin::kHandleApiConstruct, context, target, new_target,
827+
argc);
828+
}
829+
}
830+
785831
} // namespace internal
786832
} // namespace v8

‎src/builtins/builtins-definitions.h‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -236,9 +236,10 @@ namespace internal {
236236
ASM(CallApiCallbackNoSideEffects, CallApiCallbackOptimized) \
237237
ASM(CallApiCallbackWithSideEffects, CallApiCallbackOptimized) \
238238
ASM(CallApiGetter, ApiGetter) \
239-
CPP(HandleApiCall) \
240-
CPP(HandleApiCallAsFunction) \
241-
CPP(HandleApiCallAsConstructor) \
239+
TFJ(HandleApiCallOrConstruct, kDontAdaptArgumentsSentinel) \
240+
CPP(HandleApiConstruct) \
241+
CPP(HandleApiCallAsFunctionDelegate) \
242+
CPP(HandleApiCallAsConstructorDelegate) \
242243
\
243244
/* Adapters for Turbofan into runtime */ \
244245
TFC(AllocateInYoungGeneration, Allocate) \

0 commit comments

Comments
 (0)