From 4e9ac5066cb317e0330c16afea8468c3340c1987 Mon Sep 17 00:00:00 2001 From: Michael Starzinger Date: Wed, 27 Nov 2019 17:02:28 +0100 Subject: [PATCH] [turbofan] Remove unsafe {CodeAssembler::ReturnRaw}. This removes the aforementioned untyped method and switches all users to the typed TNode<> version. Those versions now contain proper checks to compare the static information against the return count and types stored in the call descriptor. R=leszeks@chromium.org BUG=v8:10021 Change-Id: I393ea6211babc100e007fb1678877d36efa7bbf7 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1939753 Reviewed-by: Tobias Tebbi Reviewed-by: Leszek Swirski Commit-Queue: Michael Starzinger Cr-Commit-Position: refs/heads/master@{#65225} --- src/builtins/builtins-bigint-gen.cc | 5 +-- src/builtins/builtins-wasm-gen.cc | 11 +++-- src/compiler/code-assembler.cc | 50 ++++++++++++++++++--- src/compiler/code-assembler.h | 6 ++- test/cctest/compiler/test-run-retpoline.cc | 4 +- test/cctest/compiler/test-run-tail-calls.cc | 2 +- 6 files changed, 57 insertions(+), 21 deletions(-) diff --git a/src/builtins/builtins-bigint-gen.cc b/src/builtins/builtins-bigint-gen.cc index 691ec7f8ce..f8fe460c45 100644 --- a/src/builtins/builtins-bigint-gen.cc +++ b/src/builtins/builtins-bigint-gen.cc @@ -25,7 +25,7 @@ TF_BUILTIN(BigIntToI64, CodeStubAssembler) { TVARIABLE(UintPtrT, var_high); BigIntToRawBytes(n, &var_low, &var_high); - ReturnRaw(var_low.value()); + Return(var_low.value()); } // https://tc39.github.io/proposal-bigint/#sec-to-big-int64 @@ -43,8 +43,7 @@ TF_BUILTIN(BigIntToI32Pair, CodeStubAssembler) { TVARIABLE(UintPtrT, var_high); BigIntToRawBytes(bigint, &var_low, &var_high); - Return(SloppyTNode(var_low.value()), - SloppyTNode(var_high.value())); + Return(var_low.value(), var_high.value()); } // https://tc39.github.io/proposal-bigint/#sec-bigint-constructor-number-value diff --git a/src/builtins/builtins-wasm-gen.cc b/src/builtins/builtins-wasm-gen.cc index 1c421f6573..a6b69176a2 100644 --- a/src/builtins/builtins-wasm-gen.cc +++ b/src/builtins/builtins-wasm-gen.cc @@ -81,7 +81,7 @@ TF_BUILTIN(WasmAtomicNotify, WasmBuiltinsAssembler) { TNode result_smi = UncheckedCast(CallRuntime(Runtime::kWasmAtomicNotify, context, instance, address_number, count_number)); - ReturnRaw(SmiToInt32(result_smi)); + Return(Unsigned(SmiToInt32(result_smi))); } TF_BUILTIN(WasmI32AtomicWait, WasmBuiltinsAssembler) { @@ -101,7 +101,7 @@ TF_BUILTIN(WasmI32AtomicWait, WasmBuiltinsAssembler) { TNode result_smi = UncheckedCast( CallRuntime(Runtime::kWasmI32AtomicWait, context, instance, address_number, expected_value_number, timeout_number)); - ReturnRaw(SmiToInt32(result_smi)); + Return(Unsigned(SmiToInt32(result_smi))); } TF_BUILTIN(WasmI64AtomicWait, WasmBuiltinsAssembler) { @@ -126,7 +126,7 @@ TF_BUILTIN(WasmI64AtomicWait, WasmBuiltinsAssembler) { TNode result_smi = UncheckedCast(CallRuntime( Runtime::kWasmI64AtomicWait, context, instance, address_number, expected_value_high_number, expected_value_low_number, timeout_number)); - ReturnRaw(SmiToInt32(result_smi)); + Return(Unsigned(SmiToInt32(result_smi))); } TF_BUILTIN(WasmMemoryGrow, WasmBuiltinsAssembler) { @@ -143,11 +143,10 @@ TF_BUILTIN(WasmMemoryGrow, WasmBuiltinsAssembler) { TNode context = LoadContextFromInstance(instance); TNode ret_smi = UncheckedCast( CallRuntime(Runtime::kWasmMemoryGrow, context, instance, num_pages_smi)); - TNode ret = SmiToInt32(ret_smi); - ReturnRaw(ret); + Return(SmiToInt32(ret_smi)); BIND(&num_pages_out_of_range); - ReturnRaw(Int32Constant(-1)); + Return(Int32Constant(-1)); } TF_BUILTIN(WasmTableGet, WasmBuiltinsAssembler) { diff --git a/src/compiler/code-assembler.cc b/src/compiler/code-assembler.cc index ff55596315..00c7cf787a 100644 --- a/src/compiler/code-assembler.cc +++ b/src/compiler/code-assembler.cc @@ -382,24 +382,64 @@ TNode CodeAssembler::GetJSContextParameter() { } void CodeAssembler::Return(SloppyTNode value) { - // TODO(leszeks): This could also return a non-object, depending on the call - // descriptor. We should probably have multiple return overloads with - // different TNode types which DCHECK the call descriptor. + DCHECK_EQ(1, raw_assembler()->call_descriptor()->ReturnCount()); + DCHECK(raw_assembler()->call_descriptor()->GetReturnType(0).IsTagged()); return raw_assembler()->Return(value); } void CodeAssembler::Return(SloppyTNode value1, SloppyTNode value2) { + DCHECK_EQ(2, raw_assembler()->call_descriptor()->ReturnCount()); + DCHECK(raw_assembler()->call_descriptor()->GetReturnType(0).IsTagged()); + DCHECK(raw_assembler()->call_descriptor()->GetReturnType(1).IsTagged()); return raw_assembler()->Return(value1, value2); } void CodeAssembler::Return(SloppyTNode value1, SloppyTNode value2, SloppyTNode value3) { + DCHECK_EQ(3, raw_assembler()->call_descriptor()->ReturnCount()); + DCHECK(raw_assembler()->call_descriptor()->GetReturnType(0).IsTagged()); + DCHECK(raw_assembler()->call_descriptor()->GetReturnType(1).IsTagged()); + DCHECK(raw_assembler()->call_descriptor()->GetReturnType(2).IsTagged()); return raw_assembler()->Return(value1, value2, value3); } +void CodeAssembler::Return(TNode value) { + DCHECK_EQ(1, raw_assembler()->call_descriptor()->ReturnCount()); + DCHECK_EQ(MachineType::Int32(), + raw_assembler()->call_descriptor()->GetReturnType(0)); + return raw_assembler()->Return(value); +} + +void CodeAssembler::Return(TNode value) { + DCHECK_EQ(1, raw_assembler()->call_descriptor()->ReturnCount()); + DCHECK_EQ(MachineType::Uint32(), + raw_assembler()->call_descriptor()->GetReturnType(0)); + return raw_assembler()->Return(value); +} + +void CodeAssembler::Return(TNode value) { + DCHECK_EQ(1, raw_assembler()->call_descriptor()->ReturnCount()); + DCHECK_EQ( + MachineType::PointerRepresentation(), + raw_assembler()->call_descriptor()->GetReturnType(0).representation()); + return raw_assembler()->Return(value); +} + +void CodeAssembler::Return(TNode value1, TNode value2) { + DCHECK_EQ(2, raw_assembler()->call_descriptor()->ReturnCount()); + DCHECK_EQ( + MachineType::PointerRepresentation(), + raw_assembler()->call_descriptor()->GetReturnType(0).representation()); + DCHECK_EQ( + MachineType::PointerRepresentation(), + raw_assembler()->call_descriptor()->GetReturnType(1).representation()); + return raw_assembler()->Return(value1, value2); +} + void CodeAssembler::PopAndReturn(Node* pop, Node* value) { + DCHECK_EQ(1, raw_assembler()->call_descriptor()->ReturnCount()); return raw_assembler()->PopAndReturn(pop, value); } @@ -411,10 +451,6 @@ void CodeAssembler::ReturnIf(Node* condition, Node* value) { Bind(&if_continue); } -void CodeAssembler::ReturnRaw(Node* value) { - return raw_assembler()->Return(value); -} - void CodeAssembler::AbortCSAAssert(Node* message) { raw_assembler()->AbortCSAAssert(message); } diff --git a/src/compiler/code-assembler.h b/src/compiler/code-assembler.h index f8cbee4f70..7c740a847a 100644 --- a/src/compiler/code-assembler.h +++ b/src/compiler/code-assembler.h @@ -543,12 +543,14 @@ class V8_EXPORT_PRIVATE CodeAssembler { void Return(SloppyTNode value1, SloppyTNode value2); void Return(SloppyTNode value1, SloppyTNode value2, SloppyTNode value3); + void Return(TNode value); + void Return(TNode value); + void Return(TNode); + void Return(TNode value1, TNode value2); void PopAndReturn(Node* pop, Node* value); void ReturnIf(Node* condition, Node* value); - void ReturnRaw(Node* value); - void AbortCSAAssert(Node* message); void DebugBreak(); void Unreachable(); diff --git a/test/cctest/compiler/test-run-retpoline.cc b/test/cctest/compiler/test-run-retpoline.cc index 32569eaaee..33f0b0bbeb 100644 --- a/test/cctest/compiler/test-run-retpoline.cc +++ b/test/cctest/compiler/test-run-retpoline.cc @@ -25,7 +25,7 @@ Handle BuildCallee(Isolate* isolate, CallDescriptor* call_descriptor) { CodeAssemblerTester tester(isolate, call_descriptor, "callee"); CodeStubAssembler assembler(tester.state()); int param_count = static_cast(call_descriptor->StackParameterCount()); - Node* sum = __ IntPtrConstant(0); + TNode sum = __ IntPtrConstant(0); for (int i = 0; i < param_count; ++i) { TNode product = __ Signed(__ IntPtrMul(__ Parameter(i), __ IntPtrConstant(i + 1))); @@ -70,7 +70,7 @@ Handle BuildCaller(Isolate* isolate, CallDescriptor* call_descriptor, } else { Node* result = tester.raw_assembler_for_testing()->CallN( callee_descriptor, param_count + 1, params.data()); - __ Return(result); + __ Return(__ UncheckedCast(result)); } return tester.GenerateCodeCloseAndEscape(); } diff --git a/test/cctest/compiler/test-run-tail-calls.cc b/test/cctest/compiler/test-run-tail-calls.cc index ed8a099090..cb83d899ed 100644 --- a/test/cctest/compiler/test-run-tail-calls.cc +++ b/test/cctest/compiler/test-run-tail-calls.cc @@ -26,7 +26,7 @@ Handle BuildCallee(Isolate* isolate, CallDescriptor* call_descriptor) { CodeAssemblerTester tester(isolate, call_descriptor, "callee"); CodeStubAssembler assembler(tester.state()); int param_count = static_cast(call_descriptor->StackParameterCount()); - Node* sum = __ IntPtrConstant(0); + TNode sum = __ IntPtrConstant(0); for (int i = 0; i < param_count; ++i) { TNode product = __ IntPtrMul(__ Parameter(i), __ IntPtrConstant(i + 1));