Commit c5cecf8d authored by Clemens Backes's avatar Clemens Backes Committed by Commit Bot

Revert "[wasm] Do memory.init bounds check in C++ code"

This reverts commit c7a26b13.

Reason for revert: Need to revert previous CL because it fails on MSVC: https://ci.chromium.org/p/v8/builders/ci/V8%20Win64%20-%20msvc/12805

Original change's description:
> [wasm] Do memory.init bounds check in C++ code
> 
> In the existing implementation we first did a bounds check in generated
> code, and then called a simple C++ function to do the actual copying.
> With this CL now we pass the WasmInstanceObject to the C++ function in
> addition to the memory.init parameters. Thereby we can do the bounds
> check in C++, which is much easier, less error prone, and which also
> speeds up code generation and reduces code size. Performance should not
> be worse, because we were already doing the call to C++ anyways.
> 
> R=​clemensb@chromium.org
> 
> Bug: v8:10281
> Change-Id: Ia86e1d08001a8bc7556277abeaa9208ec1128f89
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2096621
> Commit-Queue: Andreas Haas <ahaas@chromium.org>
> Reviewed-by: Clemens Backes <clemensb@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#66656}

TBR=ahaas@chromium.org,clemensb@chromium.org

Change-Id: I1064113e7f1c445d04652a973c994317fd3e739a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:10281
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2096624Reviewed-by: 's avatarClemens Backes <clemensb@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66657}
parent c7a26b13
...@@ -4892,21 +4892,55 @@ Node* WasmGraphBuilder::MemoryInit(uint32_t data_segment_index, Node* dst, ...@@ -4892,21 +4892,55 @@ Node* WasmGraphBuilder::MemoryInit(uint32_t data_segment_index, Node* dst,
// validation. // validation.
DCHECK_LT(data_segment_index, env_->module->num_declared_data_segments); DCHECK_LT(data_segment_index, env_->module->num_declared_data_segments);
Node* dst_fail = BoundsCheckMemRange(&dst, &size, position);
TrapIfTrue(wasm::kTrapMemOutOfBounds, dst_fail, position);
Node* seg_index = Uint32Constant(data_segment_index);
auto m = mcgraph()->machine();
{
// Load segment size from WasmInstanceObject::data_segment_sizes.
Node* seg_size_array =
LOAD_INSTANCE_FIELD(DataSegmentSizes, MachineType::Pointer());
STATIC_ASSERT(wasm::kV8MaxWasmDataSegments <= kMaxUInt32 >> 2);
Node* scaled_index = Uint32ToUintptr(
graph()->NewNode(m->Word32Shl(), seg_index, Int32Constant(2)));
Node* seg_size = SetEffect(graph()->NewNode(m->Load(MachineType::Uint32()),
seg_size_array, scaled_index,
effect(), control()));
// Bounds check the src index against the segment size.
Node* src_fail = BoundsCheckRange(src, &size, seg_size, position);
TrapIfTrue(wasm::kTrapMemOutOfBounds, src_fail, position);
}
{
// Load segment's base pointer from WasmInstanceObject::data_segment_starts.
Node* seg_start_array =
LOAD_INSTANCE_FIELD(DataSegmentStarts, MachineType::Pointer());
STATIC_ASSERT(wasm::kV8MaxWasmDataSegments <=
kMaxUInt32 / kSystemPointerSize);
Node* scaled_index = Uint32ToUintptr(graph()->NewNode(
m->Word32Shl(), seg_index, Int32Constant(kSystemPointerSizeLog2)));
Node* seg_start = SetEffect(
graph()->NewNode(m->Load(MachineType::Pointer()), seg_start_array,
scaled_index, effect(), control()));
// Convert src index to pointer.
src = graph()->NewNode(m->IntAdd(), seg_start, Uint32ToUintptr(src));
}
Node* function = graph()->NewNode(mcgraph()->common()->ExternalConstant( Node* function = graph()->NewNode(mcgraph()->common()->ExternalConstant(
ExternalReference::wasm_memory_init())); ExternalReference::wasm_memory_init()));
Node* stack_slot = StoreArgsInStackSlot( Node* stack_slot =
{{MachineType::PointerRepresentation(), instance_node_.get()}, StoreArgsInStackSlot({{MachineType::PointerRepresentation(), dst},
{MachineRepresentation::kWord32, dst}, {MachineType::PointerRepresentation(), src},
{MachineRepresentation::kWord32, src}, {MachineRepresentation::kWord32, size}});
{MachineRepresentation::kWord32,
gasm_->Uint32Constant(data_segment_index)},
{MachineRepresentation::kWord32, size}});
MachineType sig_types[] = {MachineType::Bool(), MachineType::Pointer()}; MachineType sig_types[] = {MachineType::Pointer()};
MachineSignature sig(1, 1, sig_types); MachineSignature sig(0, 1, sig_types);
Node* call = SetEffect(BuildCCall(&sig, function, stack_slot)); return SetEffect(BuildCCall(&sig, function, stack_slot));
return TrapIfFalse(wasm::kTrapMemOutOfBounds, call, position);
} }
Node* WasmGraphBuilder::DataDrop(uint32_t data_segment_index, Node* WasmGraphBuilder::DataDrop(uint32_t data_segment_index,
......
...@@ -325,31 +325,13 @@ void float64_pow_wrapper(Address data) { ...@@ -325,31 +325,13 @@ void float64_pow_wrapper(Address data) {
WriteUnalignedValue<double>(data, base::ieee754::pow(x, y)); WriteUnalignedValue<double>(data, base::ieee754::pow(x, y));
} }
bool memory_init_wrapper(Address data) { void memory_init_wrapper(Address data) {
DisallowHeapAllocation disallow_heap_allocation; Address dst = ReadUnalignedValue<Address>(data);
size_t offset = 0; Address src = ReadUnalignedValue<Address>(data + sizeof(dst));
Object raw_instance = ReadUnalignedValue<Object>(data); uint32_t size =
WasmInstanceObject instance = WasmInstanceObject::cast(raw_instance); ReadUnalignedValue<uint32_t>(data + sizeof(dst) + sizeof(src));
offset += sizeof(Object); std::memmove(reinterpret_cast<byte*>(dst), reinterpret_cast<byte*>(src),
size_t dst = ReadUnalignedValue<uint32_t>(data + offset); size);
offset += sizeof(uint32_t);
size_t src = ReadUnalignedValue<uint32_t>(data + offset);
offset += sizeof(uint32_t);
uint32_t seg_index = ReadUnalignedValue<uint32_t>(data + offset);
offset += sizeof(uint32_t);
size_t size = ReadUnalignedValue<uint32_t>(data + offset);
size_t mem_size = instance.memory_size();
if (!base::IsInBounds(dst, size, mem_size)) return false;
size_t seg_size = instance.data_segment_sizes()[seg_index];
if (!base::IsInBounds(src, size, seg_size)) return false;
byte* mem_start = instance.memory_start();
byte* seg_start =
reinterpret_cast<byte*>(instance.data_segment_starts()[seg_index]);
std::memcpy(mem_start + dst, seg_start + src, size);
return true;
} }
bool memory_copy_wrapper(Address data) { bool memory_copy_wrapper(Address data) {
......
...@@ -71,7 +71,7 @@ V8_EXPORT_PRIVATE void word64_ror_wrapper(Address data); ...@@ -71,7 +71,7 @@ V8_EXPORT_PRIVATE void word64_ror_wrapper(Address data);
V8_EXPORT_PRIVATE void float64_pow_wrapper(Address data); V8_EXPORT_PRIVATE void float64_pow_wrapper(Address data);
bool memory_init_wrapper(Address data); void memory_init_wrapper(Address data);
bool memory_copy_wrapper(Address data); bool memory_copy_wrapper(Address data);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment