Commit fabb5140 authored by Michael Starzinger's avatar Michael Starzinger Committed by Commit Bot

[wasm] Avoid embedding {null} values in WasmCode.

This loads references to {null} values from the instance object instead
of embedding them into the generated code. It is one step towards making
the {WasmCode} objects independent of the Isolate.

Note that this also fixes an issue with the serializer/deserializer that
failed to properly serialize {null} values and accidentally collapsed
them to {undefined} values instead.

R=ahaas@chromium.org
TEST=mjsunit/regress/wasm/regress-7785
BUG=v8:7424,v8:7785

Change-Id: Ie436c2d96890e7c8c89ffe2bd4189a759254775b
Reviewed-on: https://chromium-review.googlesource.com/1070981
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: 's avatarAndreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53352}
parent ee82333b
...@@ -112,15 +112,13 @@ bool ContainsInt64(wasm::FunctionSig* sig) { ...@@ -112,15 +112,13 @@ bool ContainsInt64(wasm::FunctionSig* sig) {
WasmGraphBuilder::WasmGraphBuilder( WasmGraphBuilder::WasmGraphBuilder(
Isolate* isolate, wasm::ModuleEnv* env, Zone* zone, MachineGraph* mcgraph, Isolate* isolate, wasm::ModuleEnv* env, Zone* zone, MachineGraph* mcgraph,
Handle<Code> centry_stub, Handle<Oddball> anyref_null, Handle<Code> centry_stub, wasm::FunctionSig* sig,
wasm::FunctionSig* sig,
compiler::SourcePositionTable* source_position_table) compiler::SourcePositionTable* source_position_table)
: isolate_(isolate), : isolate_(isolate),
zone_(zone), zone_(zone),
mcgraph_(mcgraph), mcgraph_(mcgraph),
env_(env), env_(env),
centry_stub_(centry_stub), centry_stub_(centry_stub),
anyref_null_(anyref_null),
cur_buffer_(def_buffer_), cur_buffer_(def_buffer_),
cur_bufsize_(kDefaultBufferSize), cur_bufsize_(kDefaultBufferSize),
has_simd_(ContainsSimd(sig)), has_simd_(ContainsSimd(sig)),
...@@ -214,11 +212,9 @@ Node* WasmGraphBuilder::EffectPhi(unsigned count, Node** effects, ...@@ -214,11 +212,9 @@ Node* WasmGraphBuilder::EffectPhi(unsigned count, Node** effects,
} }
Node* WasmGraphBuilder::RefNull() { Node* WasmGraphBuilder::RefNull() {
if (!anyref_null_node_.is_set()) { Node* null = LOAD_INSTANCE_FIELD(NullValue, MachineType::TaggedPointer());
anyref_null_node_.set( *effect_ = null;
graph()->NewNode(mcgraph()->common()->HeapConstant(anyref_null_))); return null;
}
return anyref_null_node_.get();
} }
Node* WasmGraphBuilder::CEntryStub() { Node* WasmGraphBuilder::CEntryStub() {
...@@ -3978,8 +3974,7 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder { ...@@ -3978,8 +3974,7 @@ class WasmWrapperGraphBuilder : public WasmGraphBuilder {
wasm::FunctionSig* sig, wasm::FunctionSig* sig,
compiler::SourcePositionTable* spt) compiler::SourcePositionTable* spt)
: WasmGraphBuilder(jsgraph->isolate(), env, zone, jsgraph, : WasmGraphBuilder(jsgraph->isolate(), env, zone, jsgraph,
CodeFactory::CEntry(jsgraph->isolate()), CodeFactory::CEntry(jsgraph->isolate()), sig, spt),
jsgraph->isolate()->factory()->null_value(), sig, spt),
jsgraph_(jsgraph) {} jsgraph_(jsgraph) {}
Node* BuildAllocateHeapNumberWithValue(Node* value, Node* control) { Node* BuildAllocateHeapNumberWithValue(Node* value, Node* control) {
...@@ -5007,12 +5002,8 @@ SourcePositionTable* TurbofanWasmCompilationUnit::BuildGraphForWasmFunction( ...@@ -5007,12 +5002,8 @@ SourcePositionTable* TurbofanWasmCompilationUnit::BuildGraphForWasmFunction(
// Create a TF graph during decoding. // Create a TF graph during decoding.
SourcePositionTable* source_position_table = SourcePositionTable* source_position_table =
new (mcgraph_->zone()) SourcePositionTable(mcgraph_->graph()); new (mcgraph_->zone()) SourcePositionTable(mcgraph_->graph());
// We get the handle for {null_value()} directly from the isolate although we
// are on a background task because the handle is stored in the isolate
// anyways, and it is immortal and immovable.
WasmGraphBuilder builder(wasm_unit_->isolate_, wasm_unit_->env_, WasmGraphBuilder builder(wasm_unit_->isolate_, wasm_unit_->env_,
mcgraph_->zone(), mcgraph_, wasm_unit_->centry_stub_, mcgraph_->zone(), mcgraph_, wasm_unit_->centry_stub_,
wasm_unit_->isolate_->factory()->null_value(),
wasm_unit_->func_body_.sig, source_position_table); wasm_unit_->func_body_.sig, source_position_table);
graph_construction_result_ = wasm::BuildTFGraph( graph_construction_result_ = wasm::BuildTFGraph(
wasm_unit_->isolate_->allocator(), &builder, wasm_unit_->func_body_); wasm_unit_->isolate_->allocator(), &builder, wasm_unit_->func_body_);
......
...@@ -159,7 +159,7 @@ class WasmGraphBuilder { ...@@ -159,7 +159,7 @@ class WasmGraphBuilder {
WasmGraphBuilder(Isolate* isolate, wasm::ModuleEnv* env, Zone* zone, WasmGraphBuilder(Isolate* isolate, wasm::ModuleEnv* env, Zone* zone,
MachineGraph* mcgraph, Handle<Code> centry_stub, MachineGraph* mcgraph, Handle<Code> centry_stub,
Handle<Oddball> anyref_null, wasm::FunctionSig* sig, wasm::FunctionSig* sig,
compiler::SourcePositionTable* spt = nullptr); compiler::SourcePositionTable* spt = nullptr);
Node** Buffer(size_t count) { Node** Buffer(size_t count) {
...@@ -344,13 +344,11 @@ class WasmGraphBuilder { ...@@ -344,13 +344,11 @@ class WasmGraphBuilder {
WasmInstanceCacheNodes* instance_cache_ = nullptr; WasmInstanceCacheNodes* instance_cache_ = nullptr;
Handle<Code> centry_stub_; Handle<Code> centry_stub_;
Handle<Oddball> anyref_null_;
SetOncePointer<Node> instance_node_; SetOncePointer<Node> instance_node_;
SetOncePointer<Node> globals_start_; SetOncePointer<Node> globals_start_;
SetOncePointer<Node> imported_mutable_globals_; SetOncePointer<Node> imported_mutable_globals_;
SetOncePointer<Node> centry_stub_node_; SetOncePointer<Node> centry_stub_node_;
SetOncePointer<Node> anyref_null_node_;
SetOncePointer<Node> stack_check_builtin_code_node_; SetOncePointer<Node> stack_check_builtin_code_node_;
const Operator* stack_check_call_operator_ = nullptr; const Operator* stack_check_call_operator_ = nullptr;
......
...@@ -278,7 +278,7 @@ void WasmCode::Validate() const { ...@@ -278,7 +278,7 @@ void WasmCode::Validate() const {
break; break;
case RelocInfo::EMBEDDED_OBJECT: { case RelocInfo::EMBEDDED_OBJECT: {
HeapObject* o = it.rinfo()->target_object(); HeapObject* o = it.rinfo()->target_object();
DCHECK(o->IsUndefined(o->GetIsolate()) || o->IsNull(o->GetIsolate())); DCHECK(o->IsUndefined(o->GetIsolate()));
break; break;
} }
default: default:
......
...@@ -155,6 +155,7 @@ OPTIONAL_ACCESSORS(WasmInstanceObject, managed_native_allocations, Foreign, ...@@ -155,6 +155,7 @@ OPTIONAL_ACCESSORS(WasmInstanceObject, managed_native_allocations, Foreign,
kManagedNativeAllocationsOffset) kManagedNativeAllocationsOffset)
OPTIONAL_ACCESSORS(WasmInstanceObject, managed_indirect_patcher, Foreign, OPTIONAL_ACCESSORS(WasmInstanceObject, managed_indirect_patcher, Foreign,
kManagedIndirectPatcherOffset) kManagedIndirectPatcherOffset)
ACCESSORS(WasmInstanceObject, null_value, Oddball, kNullValueOffset)
inline bool WasmInstanceObject::has_indirect_function_table() { inline bool WasmInstanceObject::has_indirect_function_table() {
return indirect_function_table_sig_ids() != nullptr; return indirect_function_table_sig_ids() != nullptr;
......
...@@ -805,6 +805,7 @@ Handle<WasmInstanceObject> WasmInstanceObject::New( ...@@ -805,6 +805,7 @@ Handle<WasmInstanceObject> WasmInstanceObject::New(
instance->set_compiled_module(*compiled_module); instance->set_compiled_module(*compiled_module);
instance->set_native_context(*isolate->native_context()); instance->set_native_context(*isolate->native_context());
instance->set_module_object(*module_object); instance->set_module_object(*module_object);
instance->set_null_value(isolate->heap()->null_value());
return instance; return instance;
} }
......
...@@ -288,6 +288,7 @@ class WasmInstanceObject : public JSObject { ...@@ -288,6 +288,7 @@ class WasmInstanceObject : public JSObject {
DECL_OPTIONAL_ACCESSORS(indirect_function_table_instances, FixedArray) DECL_OPTIONAL_ACCESSORS(indirect_function_table_instances, FixedArray)
DECL_OPTIONAL_ACCESSORS(managed_native_allocations, Foreign) DECL_OPTIONAL_ACCESSORS(managed_native_allocations, Foreign)
DECL_OPTIONAL_ACCESSORS(managed_indirect_patcher, Foreign) DECL_OPTIONAL_ACCESSORS(managed_indirect_patcher, Foreign)
DECL_ACCESSORS(null_value, Oddball)
DECL_PRIMITIVE_ACCESSORS(memory_start, byte*) DECL_PRIMITIVE_ACCESSORS(memory_start, byte*)
DECL_PRIMITIVE_ACCESSORS(memory_size, uint32_t) DECL_PRIMITIVE_ACCESSORS(memory_size, uint32_t)
DECL_PRIMITIVE_ACCESSORS(memory_mask, uint32_t) DECL_PRIMITIVE_ACCESSORS(memory_mask, uint32_t)
...@@ -318,6 +319,7 @@ class WasmInstanceObject : public JSObject { ...@@ -318,6 +319,7 @@ class WasmInstanceObject : public JSObject {
V(kIndirectFunctionTableInstancesOffset, kPointerSize) \ V(kIndirectFunctionTableInstancesOffset, kPointerSize) \
V(kManagedNativeAllocationsOffset, kPointerSize) \ V(kManagedNativeAllocationsOffset, kPointerSize) \
V(kManagedIndirectPatcherOffset, kPointerSize) \ V(kManagedIndirectPatcherOffset, kPointerSize) \
V(kNullValueOffset, kPointerSize) \
V(kFirstUntaggedOffset, 0) /* marker */ \ V(kFirstUntaggedOffset, 0) /* marker */ \
V(kMemoryStartOffset, kPointerSize) /* untagged */ \ V(kMemoryStartOffset, kPointerSize) /* untagged */ \
V(kMemorySizeOffset, kUInt32Size) /* untagged */ \ V(kMemorySizeOffset, kUInt32Size) /* untagged */ \
......
...@@ -268,21 +268,10 @@ void TestBuildingGraph(Zone* zone, compiler::JSGraph* jsgraph, ...@@ -268,21 +268,10 @@ void TestBuildingGraph(Zone* zone, compiler::JSGraph* jsgraph,
ModuleEnv* module, FunctionSig* sig, ModuleEnv* module, FunctionSig* sig,
compiler::SourcePositionTable* source_position_table, compiler::SourcePositionTable* source_position_table,
const byte* start, const byte* end) { const byte* start, const byte* end) {
if (module) { compiler::WasmGraphBuilder builder(jsgraph->isolate(), module, zone, jsgraph,
compiler::WasmGraphBuilder builder(
jsgraph->isolate(), module, zone, jsgraph,
CodeFactory::CEntry(jsgraph->isolate(), 1), CodeFactory::CEntry(jsgraph->isolate(), 1),
jsgraph->isolate()->factory()->null_value(), sig, sig, source_position_table);
source_position_table);
TestBuildingGraphWithBuilder(&builder, zone, sig, start, end); TestBuildingGraphWithBuilder(&builder, zone, sig, start, end);
} else {
compiler::WasmGraphBuilder builder(
jsgraph->isolate(), nullptr, zone, jsgraph,
CodeFactory::CEntry(jsgraph->isolate(), 1),
jsgraph->isolate()->factory()->null_value(), sig,
source_position_table);
TestBuildingGraphWithBuilder(&builder, zone, sig, start, end);
}
} }
WasmFunctionWrapper::WasmFunctionWrapper(Zone* zone, int num_params) WasmFunctionWrapper::WasmFunctionWrapper(Zone* zone, int num_params)
......
// Copyright 2018 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --allow-natives-syntax --experimental-wasm-anyref
load("test/mjsunit/wasm/wasm-constants.js");
load("test/mjsunit/wasm/wasm-module-builder.js");
(function testAnyRefNull() {
const builder = new WasmModuleBuilder();
builder.addFunction('main', kSig_r_v)
.addBody([kExprRefNull])
.exportFunc();
var wire_bytes = builder.toBuffer();
var module = new WebAssembly.Module(wire_bytes);
var buffer = %SerializeWasmModule(module);
module = %DeserializeWasmModule(buffer, wire_bytes);
var instance = new WebAssembly.Instance(module);
assertEquals(null, instance.exports.main());
})();
(function testAnyRefIsNull() {
const builder = new WasmModuleBuilder();
builder.addFunction('main', kSig_i_r)
.addBody([kExprGetLocal, 0, kExprRefIsNull])
.exportFunc();
var wire_bytes = builder.toBuffer();
var module = new WebAssembly.Module(wire_bytes);
var buffer = %SerializeWasmModule(module);
module = %DeserializeWasmModule(buffer, wire_bytes);
var instance = new WebAssembly.Instance(module);
assertEquals(0, instance.exports.main({'hello' : 'world'}));
assertEquals(0, instance.exports.main(1234));
assertEquals(0, instance.exports.main(0));
assertEquals(0, instance.exports.main(123.4));
assertEquals(0, instance.exports.main(undefined));
assertEquals(1, instance.exports.main(null));
assertEquals(0, instance.exports.main(print));
})();
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