Skip to content

Commit dcfc453

Browse files
tebbiCommit Bot
tebbi
authored and
Commit Bot
committed
[csa] check arity when calling
Add information to CallInterfaceDescriptor if additional implicit arguments can be passed on the stack, that is, if it is a varargs calling convention. With this information, we can have a proper DCHECK in CSA to avoid passing the wrong number of arguments to builtins that don't support it. Previously, this lead to difficult to investigate crashes with misaligned stacks. Drive-by cleanup: Reduce duplication between DEFINE_PARAMETERS_... macros. Change-Id: I449af6713a3cdd72e098d3481dfee62e01343f14 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1888932 Reviewed-by: Jakob Gruber <jgruber@chromium.org> Reviewed-by: Igor Sheludko <ishell@chromium.org> Commit-Queue: Tobias Tebbi <tebbi@chromium.org> Cr-Commit-Position: refs/heads/master@{#64666}
1 parent dba8629 commit dcfc453

2 files changed

Lines changed: 43 additions & 30 deletions

File tree

‎src/codegen/interface-descriptors.h‎

Lines changed: 36 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -103,11 +103,14 @@ class V8_EXPORT_PRIVATE CallInterfaceDescriptorData {
103103
enum Flag {
104104
kNoFlags = 0u,
105105
kNoContext = 1u << 0,
106-
107106
// This indicates that the code uses a special frame that does not scan the
108107
// stack arguments, e.g. EntryFrame. And this allows the code to use
109108
// untagged stack arguments.
110109
kNoStackScan = 1u << 1,
110+
// In addition to the specified parameters, additional arguments can be
111+
// passed on the stack.
112+
// This does not indicate if arguments adaption is used or not.
113+
kAllowVarArgs = 1u << 2,
111114
};
112115
using Flags = base::Flags<Flag>;
113116

@@ -249,6 +252,10 @@ class V8_EXPORT_PRIVATE CallInterfaceDescriptor {
249252
return (flags() & CallInterfaceDescriptorData::kNoContext) == 0;
250253
}
251254

255+
bool AllowVarArgs() const {
256+
return flags() & CallInterfaceDescriptorData::kAllowVarArgs;
257+
}
258+
252259
int GetReturnCount() const { return data()->return_count(); }
253260

254261
MachineType GetReturnType(int index) const {
@@ -400,28 +407,20 @@ STATIC_ASSERT(kMaxTFSBuiltinRegisterParams <= kMaxBuiltinRegisterParams);
400407
\
401408
public:
402409

403-
#define DEFINE_RESULT_AND_PARAMETERS(return_count, ...) \
404-
static constexpr int kDescriptorFlags = \
405-
CallInterfaceDescriptorData::kNoFlags; \
406-
static constexpr int kReturnCount = return_count; \
407-
enum ParameterIndices { \
408-
__dummy = -1, /* to be able to pass zero arguments */ \
409-
##__VA_ARGS__, \
410-
\
411-
kParameterCount, \
412-
kContext = kParameterCount /* implicit parameter */ \
410+
#define DEFINE_FLAGS_AND_RESULT_AND_PARAMETERS(flags, return_count, ...) \
411+
static constexpr int kDescriptorFlags = flags; \
412+
static constexpr int kReturnCount = return_count; \
413+
enum ParameterIndices { \
414+
__dummy = -1, /* to be able to pass zero arguments */ \
415+
##__VA_ARGS__, \
416+
\
417+
kParameterCount, \
418+
kContext = kParameterCount /* implicit parameter */ \
413419
};
414420

415-
#define DEFINE_RESULT_AND_PARAMETERS_NO_CONTEXT(return_count, ...) \
416-
static constexpr int kDescriptorFlags = \
417-
CallInterfaceDescriptorData::kNoContext; \
418-
static constexpr int kReturnCount = return_count; \
419-
enum ParameterIndices { \
420-
__dummy = -1, /* to be able to pass zero arguments */ \
421-
##__VA_ARGS__, \
422-
\
423-
kParameterCount \
424-
};
421+
#define DEFINE_RESULT_AND_PARAMETERS(return_count, ...) \
422+
DEFINE_FLAGS_AND_RESULT_AND_PARAMETERS( \
423+
CallInterfaceDescriptorData::kNoFlags, return_count, ##__VA_ARGS__)
425424

426425
// This is valid only for builtins that use EntryFrame, which does not scan
427426
// stack arguments on GC.
@@ -437,10 +436,17 @@ STATIC_ASSERT(kMaxTFSBuiltinRegisterParams <= kMaxBuiltinRegisterParams);
437436
kParameterCount \
438437
};
439438

440-
#define DEFINE_PARAMETERS(...) DEFINE_RESULT_AND_PARAMETERS(1, ##__VA_ARGS__)
439+
#define DEFINE_PARAMETERS(...) \
440+
DEFINE_FLAGS_AND_RESULT_AND_PARAMETERS( \
441+
CallInterfaceDescriptorData::kNoFlags, 1, ##__VA_ARGS__)
441442

442443
#define DEFINE_PARAMETERS_NO_CONTEXT(...) \
443-
DEFINE_RESULT_AND_PARAMETERS_NO_CONTEXT(1, ##__VA_ARGS__)
444+
DEFINE_FLAGS_AND_RESULT_AND_PARAMETERS( \
445+
CallInterfaceDescriptorData::kNoContext, 1, ##__VA_ARGS__)
446+
447+
#define DEFINE_PARAMETERS_VARARGS(...) \
448+
DEFINE_FLAGS_AND_RESULT_AND_PARAMETERS( \
449+
CallInterfaceDescriptorData::kAllowVarArgs, 1, ##__VA_ARGS__)
444450

445451
#define DEFINE_RESULT_AND_PARAMETER_TYPES(...) \
446452
void InitializePlatformIndependent(CallInterfaceDescriptorData* data) \
@@ -460,7 +466,7 @@ STATIC_ASSERT(kMaxTFSBuiltinRegisterParams <= kMaxBuiltinRegisterParams);
460466

461467
#define DEFINE_JS_PARAMETERS(...) \
462468
static constexpr int kDescriptorFlags = \
463-
CallInterfaceDescriptorData::kNoFlags; \
469+
CallInterfaceDescriptorData::kAllowVarArgs; \
464470
static constexpr int kReturnCount = 1; \
465471
enum ParameterIndices { \
466472
kTarget, \
@@ -843,7 +849,7 @@ class TypeofDescriptor : public CallInterfaceDescriptor {
843849

844850
class CallTrampolineDescriptor : public CallInterfaceDescriptor {
845851
public:
846-
DEFINE_PARAMETERS(kFunction, kActualArgumentsCount)
852+
DEFINE_PARAMETERS_VARARGS(kFunction, kActualArgumentsCount)
847853
DEFINE_PARAMETER_TYPES(MachineType::AnyTagged(), // kFunction
848854
MachineType::Int32()) // kActualArgumentsCount
849855
DECLARE_DESCRIPTOR(CallTrampolineDescriptor, CallInterfaceDescriptor)
@@ -871,7 +877,7 @@ class CallForwardVarargsDescriptor : public CallInterfaceDescriptor {
871877

872878
class CallFunctionTemplateDescriptor : public CallInterfaceDescriptor {
873879
public:
874-
DEFINE_PARAMETERS(kFunctionTemplateInfo, kArgumentsCount)
880+
DEFINE_PARAMETERS_VARARGS(kFunctionTemplateInfo, kArgumentsCount)
875881
DEFINE_PARAMETER_TYPES(MachineType::AnyTagged(), // kFunctionTemplateInfo
876882
MachineType::IntPtr()) // kArgumentsCount
877883
DECLARE_DESCRIPTOR(CallFunctionTemplateDescriptor, CallInterfaceDescriptor)
@@ -1087,8 +1093,8 @@ class CEntry1ArgvOnStackDescriptor : public CallInterfaceDescriptor {
10871093

10881094
class ApiCallbackDescriptor : public CallInterfaceDescriptor {
10891095
public:
1090-
DEFINE_PARAMETERS(kApiFunctionAddress, kActualArgumentsCount, kCallData,
1091-
kHolder)
1096+
DEFINE_PARAMETERS_VARARGS(kApiFunctionAddress, kActualArgumentsCount,
1097+
kCallData, kHolder)
10921098
// receiver is implicit stack argument 1
10931099
// argv are implicit stack arguments [2, 2 + kArgc[
10941100
DEFINE_PARAMETER_TYPES(MachineType::Pointer(), // kApiFunctionAddress
@@ -1364,9 +1370,10 @@ BUILTIN_LIST_TFS(DEFINE_TFS_BUILTIN_DESCRIPTOR)
13641370
#undef DECLARE_DESCRIPTOR_WITH_BASE
13651371
#undef DECLARE_DESCRIPTOR
13661372
#undef DECLARE_JS_COMPATIBLE_DESCRIPTOR
1373+
#undef DEFINE_FLAGS_AND_RESULT_AND_PARAMETERS
13671374
#undef DEFINE_RESULT_AND_PARAMETERS
1368-
#undef DEFINE_RESULT_AND_PARAMETERS_NO_CONTEXT
13691375
#undef DEFINE_PARAMETERS
1376+
#undef DEFINE_PARAMETERS_VARARGS
13701377
#undef DEFINE_PARAMETERS_NO_CONTEXT
13711378
#undef DEFINE_RESULT_AND_PARAMETER_TYPES
13721379
#undef DEFINE_PARAMETER_TYPES

‎src/compiler/code-assembler.cc‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1310,7 +1310,13 @@ Node* CodeAssembler::CallStubN(StubCallMode call_mode,
13101310
int implicit_nodes = descriptor.HasContextParameter() ? 2 : 1;
13111311
DCHECK_LE(implicit_nodes, input_count);
13121312
int argc = input_count - implicit_nodes;
1313-
DCHECK_LE(descriptor.GetParameterCount(), argc);
1313+
#ifdef DEBUG
1314+
if (descriptor.AllowVarArgs()) {
1315+
DCHECK_LE(descriptor.GetParameterCount(), argc);
1316+
} else {
1317+
DCHECK_EQ(descriptor.GetParameterCount(), argc);
1318+
}
1319+
#endif
13141320
// Extra arguments not mentioned in the descriptor are passed on the stack.
13151321
int stack_parameter_count = argc - descriptor.GetRegisterParameterCount();
13161322
DCHECK_LE(descriptor.GetStackParameterCount(), stack_parameter_count);

0 commit comments

Comments
 (0)