Skip to content

Commit

Permalink
Revert of MIPS[64]: Fix unaligned arguments storage in Wasm-to-interp…
Browse files Browse the repository at this point in the history
…reter entry (patchset crosswalk-project#3 id:40001 of https://codereview.chromium.org/2705293011/ )

Reason for revert:
Did not fix the issue.

Original issue's description:
> MIPS[64]: Fix unaligned arguments storage in Wasm-to-interpreter entry
>
> In Wasm-to-interpeter entry creation, arguments for the interpreter
> are stored in an argument buffer. Depending on the order of the
> arguments some arguments may be misaligned and this causes crashes
> on those architectures that do not support unaligned memory access.
>
> TEST=cctest/test-wasm-interpreter-entry/TestArgumentPassing_AllTypes
> BUG=
>
> Review-Url: https://codereview.chromium.org/2705293011
> Cr-Commit-Position: refs/heads/master@{#43476}
> Committed: https://chromium.googlesource.com/v8/v8/+/84ff6e4c1997b63c01e95504c31ee6c5504430d5

[email protected],[email protected]
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=

Review-Url: https://codereview.chromium.org/2760603002
Cr-Commit-Position: refs/heads/master@{#43893}
  • Loading branch information
backes authored and Commit bot committed Mar 17, 2017
1 parent 876725d commit 29877f5
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 18 deletions.
12 changes: 3 additions & 9 deletions src/compiler/wasm-compiler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2761,8 +2761,7 @@ void WasmGraphBuilder::BuildWasmInterpreterEntry(
// Compute size for the argument buffer.
int args_size_bytes = 0;
for (int i = 0; i < wasm_count; i++) {
args_size_bytes +=
RoundUpToMultipleOfPowOf2(1 << ElementSizeLog2Of(sig->GetParam(i)), 8);
args_size_bytes += 1 << ElementSizeLog2Of(sig->GetParam(i));
}

// The return value is also passed via this buffer:
Expand Down Expand Up @@ -2807,10 +2806,8 @@ void WasmGraphBuilder::BuildWasmInterpreterEntry(
*effect_ =
graph()->NewNode(jsgraph()->machine()->Store(store_rep), arg_buffer,
Int32Constant(offset), param, *effect_, *control_);
offset += RoundUpToMultipleOfPowOf2(1 << ElementSizeLog2Of(param_rep), 8);
offset += 1 << ElementSizeLog2Of(param_rep);
}

DCHECK(IsAligned(offset, 8));
}
DCHECK_EQ(param_count, param_index);
DCHECK_EQ(args_size_bytes, offset);
Expand Down Expand Up @@ -3788,10 +3785,7 @@ Handle<Code> CompileWasmInterpreterEntry(Isolate* isolate, uint32_t func_index,
Zone zone(isolate->allocator(), ZONE_NAME);
Graph graph(&zone);
CommonOperatorBuilder common(&zone);
MachineOperatorBuilder machine(
&zone, MachineType::PointerRepresentation(),
InstructionSelector::SupportedMachineOperatorFlags(),
InstructionSelector::AlignmentRequirements());
MachineOperatorBuilder machine(&zone);
JSGraph jsgraph(isolate, &graph, &common, nullptr, nullptr, &machine);

Node* control = nullptr;
Expand Down
5 changes: 0 additions & 5 deletions src/utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -192,11 +192,6 @@ inline bool IsAddressAligned(Address addr,
return IsAligned(offs, alignment);
}

template <typename T, typename U>
inline T RoundUpToMultipleOfPowOf2(T value, U multiple) {
DCHECK(multiple && ((multiple & (multiple - 1)) == 0));
return (value + multiple - 1) & ~(multiple - 1);
}

// Returns the maximum of the two parameters.
template <typename T>
Expand Down
8 changes: 4 additions & 4 deletions src/wasm/wasm-debug.cc
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,11 @@ class InterpreterHandle {
ScopedVector<WasmVal> wasm_args(num_params);
uint8_t* arg_buf_ptr = arg_buffer;
for (int i = 0; i < num_params; ++i) {
int param_size = 1 << ElementSizeLog2Of(sig->GetParam(i));
uint32_t param_size = 1 << ElementSizeLog2Of(sig->GetParam(i));
#define CASE_ARG_TYPE(type, ctype) \
case type: \
DCHECK_EQ(param_size, sizeof(ctype)); \
wasm_args[i] = WasmVal(*reinterpret_cast<ctype*>(arg_buf_ptr)); \
wasm_args[i] = WasmVal(ReadUnalignedValue<ctype>(arg_buf_ptr)); \
break;
switch (sig->GetParam(i)) {
CASE_ARG_TYPE(kWasmI32, uint32_t)
Expand All @@ -148,7 +148,7 @@ class InterpreterHandle {
default:
UNREACHABLE();
}
arg_buf_ptr += RoundUpToMultipleOfPowOf2(param_size, 8);
arg_buf_ptr += param_size;
}

WasmInterpreter::Thread* thread = interpreter_.GetThread(0);
Expand Down Expand Up @@ -197,7 +197,7 @@ class InterpreterHandle {
#define CASE_RET_TYPE(type, ctype) \
case type: \
DCHECK_EQ(1 << ElementSizeLog2Of(sig->GetReturn(0)), sizeof(ctype)); \
*reinterpret_cast<ctype*>(arg_buffer) = ret_val.to<ctype>(); \
WriteUnalignedValue<ctype>(arg_buffer, ret_val.to<ctype>()); \
break;
switch (sig->GetReturn(0)) {
CASE_RET_TYPE(kWasmI32, uint32_t)
Expand Down

0 comments on commit 29877f5

Please sign in to comment.